audit: money-correctness fixes — DO NOT MERGE before ACTION_REQUIRED.md §0

.dockerignore

@@ -6,3 +6,13 @@
 docs
 screenshots
 *.md
+# Never let a developer's local secrets into the build context. `COPY server/ ./` and
+# `COPY web/ ./` would otherwise bake a local .env — with real Stripe keys in it — into an
+# image layer, where it survives even if a later step deletes the file (DONATIONS-027).
+.env
+**/.env
+**/.env.*
+*.pem
+*.key
+*.p12
+*.pfx

.github/workflows/audit.yml

@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: AGPL-3.0-only
+# Copyright (C) 2026 OpenMasjid-Solutions
+
+name: Dependency audit
+
+# Before this existed, the ONLY workflow in the repo was the release image build — so a new advisory
+# against fastify, better-sqlite3 or the Stripe SDK went unnoticed until somebody happened to run
+# `npm audit` by hand. This app handles donations on unattended Raspberry Pis, so "nobody looked for
+# six months" is the realistic failure mode (DONATIONS-043).
+#
+# Deliberately read-only and unprivileged: it reports, it never opens a PR, never pushes, never
+# touches the registry, and holds no secret beyond the default read token. It cannot become a
+# supply-chain path itself.
+on:
+  schedule:
+    - cron: '17 6 * * 1' # Mondays, 06:17 UTC — off the hour, so not in the every-runner rush
+  pull_request:
+    paths:
+      - '**/package.json'
+      - '**/package-lock.json'
+      - '.github/workflows/audit.yml'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  audit:
+    runs-on: ubuntu-latest
+    steps:
+      # Pinned to a commit SHA, like every other Action in this repo (DONATIONS-004).
+      - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+      - uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 # v6.0.0
+        with:
+          node-version: '22'
+
+      # `--omit=dev` first: a HIGH advisory in something that ships to a masjid box is a different
+      # thing from one in a build-time devDependency, and only the first should fail the run.
+      # `|| true` on the second so a dev-only advisory is reported but not treated as a release
+      # blocker — it is still visible in the log and in the summary below.
+      - name: Audit production dependencies (fails the run)
+        run: |
+          echo "## Server — production dependencies" >> "$GITHUB_STEP_SUMMARY"
+          cd server && npm audit --omit=dev --audit-level=high 2>&1 | tee -a "$GITHUB_STEP_SUMMARY"
+
+      - name: Audit everything else (report only)
+        if: always()
+        run: |
+          {
+            echo "## Server — including devDependencies"
+            (cd server && npm audit || true)
+            echo "## Web — including devDependencies"
+            (cd web && npm audit || true)
+          } >> "$GITHUB_STEP_SUMMARY"

.github/workflows/build-image.yml

@@ -35,8 +35,15 @@ permissions:
 jobs:
   build:
     runs-on: ubuntu-latest
+    # Every third-party Action is pinned to an immutable commit SHA, not a moving major tag
+    # (DONATIONS-004). This job holds `packages: write` and a GHCR credential for the image every
+    # masjid Raspberry Pi pulls, so an upstream owner (or anyone who compromises one of these
+    # repos) repointing `v6` would run their code next to a live publish token. cla.yml already
+    # pins its action this way; this file did not. Each SHA below is exactly what the tag it
+    # replaces resolved to on 2026-08-03 — nothing was upgraded, only frozen.
+    # To update: read the release notes, then `gh api repos/<owner>/<repo>/git/ref/tags/<tag>`.
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
 
       - name: Resolve image name + version
         id: meta
@@ -47,11 +54,11 @@ jobs:
           echo "owner=${GITHUB_REPOSITORY_OWNER,,}" >> "$GITHUB_OUTPUT"
           echo "name=$(echo "${GITHUB_REPOSITORY##*/}" | tr '[:upper:]' '[:lower:]')" >> "$GITHUB_OUTPUT"
 
-      - uses: docker/setup-qemu-action@v3
-      - uses: docker/setup-buildx-action@v3
+      - uses: docker/setup-qemu-action@c7c53464625b32c7a7e944ae62b3e17d2b600130 # v3.7.0
+      - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0
 
       - name: Log in to GHCR
-        uses: docker/login-action@v3
+        uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0
         with:
           registry: ghcr.io
           username: ${{ github.actor }}
@@ -61,7 +68,7 @@ jobs:
       # OpenMasjidDisplay template. Bump manifest.yaml `version:` for each meaningful
       # change so a pinned install stays stable (the catalog pins a git tag + version).
       - name: Build and push
-        uses: docker/build-push-action@v6
+        uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2
         with:
           context: .
           platforms: linux/amd64,linux/arm64

CLAUDE.md

@@ -177,7 +177,7 @@ resources:
 ## 11. Tech stack (match Display)
 
 - **TypeScript everywhere.** `strict` on, no `any` without a justifying comment.
-- **`server/`** — Node 20+ + **Fastify** REST API (WebSocket only if you actually need live updates; donations probably don't). **better-sqlite3** for storage. **`stripe`** SDK. **argon2** for the fallback admin password. Validate input with **zod**.
+- **`server/`** — Node 20+ + **Fastify** REST API (WebSocket only if you actually need live updates; donations probably don't). **better-sqlite3** for storage. **`stripe`** SDK. **scrypt** (Node built-in, N=2^16) for the fallback admin password — no external crypto dependency. Validate input with **zod**.
 - **`web/`** — **React + Vite + TypeScript + Tailwind**, **shadcn/ui** components, **Motion** for animation, **lucide-react** icons, **@stripe/react-stripe-js** for the Payment Element. One app serving the public site and the `/admin` panel.
 - **One container** via a multi-stage **Dockerfile** (build web, build server, final runtime serves the web build + API), exactly like Display. `docker compose up -d` runs it.
 - Keep it **lean and Pi-friendly**; lazy-load the admin bundle so the donor page stays light.

docs/audit/ACTION_REQUIRED.md

@@ -0,0 +1,220 @@
+<!-- SPDX-License-Identifier: AGPL-3.0-only -->
+<!-- Copyright (C) 2026 OpenMasjid-Solutions -->
+
+# Action required — only you can do these
+
+From the 2026-08-03 security and code-health audit. Ordered by urgency.
+
+---
+
+## 0. READ FIRST — money may have been mischarged
+
+### 0a. Three-decimal currencies charge one tenth (DONATIONS-001)
+
+**Do this before merging the money PR.**
+
+If any masjid running this app is configured in **BHD, JOD, KWD, OMR or TND**, every donation it has
+ever taken charged **one tenth** of the amount the donor was shown, while the local ledger recorded
+the full amount. The app agreed with itself, so nothing looked wrong; only the Stripe dashboard has
+the true figures.
+
+**What I need you to do, in order:**
+
+1. **Find out whether this is live.** Check each installation's configured currency (Settings →
+   currency, or the `CURRENCY` / `MASJID_CURRENCY` env var). If none is one of those five, this is
+   latent — merge the fix and move on.
+2. **If any masjid IS in one of those five**, before merging: export their donation history from the
+   **Stripe dashboard** and compare it with the app's CSV. Stripe is the truth. Expect app figures to
+   be 10× the real amounts.
+3. **Decide what happens to the historical rows.** The fix changes only future conversions; it does
+   not rewrite stored amounts, so after merging, old rows (stored as 1/10 scale) and new rows (full
+   scale) will mean different things in the same column. Options, all yours: leave them and annotate
+   the period; or write a one-off migration multiplying affected rows by 10. **I have not written
+   that migration** — it edits historical financial records and needs someone who can confirm the
+   affected date range against Stripe first.
+4. **Talk to the community if donors were undercharged.** A donor who gave "100 KWD" of Zakat paid
+   10. That is a religious obligation they may believe is discharged. Whether and how to tell them is
+   not a decision code should make.
+
+Fix is written and tested on branch `audit/money-2026-08-03`, commit `91767c6`. Two-decimal
+currencies are bit-identical — verified.
+
+### 0b. One-time donations have been silently lost (DONATIONS-002)
+
+A card payment that succeeded at Stripe while the donor's `/confirm` callback failed (closed tab,
+lost signal, box briefly unreachable) was **never recorded, never receipted, and never counted**. The
+row sits at `pending` for ever, indistinguishable from an abandoned checkout.
+
+**What I need you to do:**
+
+1. **Find out whether it has happened.** For each installation, compare succeeded PaymentIntents in
+   Stripe against the app's ledger for the same period. Any Stripe `succeeded` intent with no
+   corresponding donation is a lost donation. The app-side signal is a `pending` row more than a few
+   minutes old.
+2. **Expect the ledger to grow when the fix merges.** The sweep will find those payments and add
+   them, backdated. Totals, the CSV, the trend chart and any Gift Aid claim will all increase. That
+   is correct — the money did arrive — but if a masjid has already filed accounts or a Gift Aid claim
+   on the old figures, they need to know before the numbers move.
+3. **Expect receipt emails to go out** to donors whose payment was recovered and who never got one.
+   Some may be months old. If that is not wanted, disable receipts in Settings before deploying, or
+   ask me to gate the sweep's receipt behind an age limit.
+
+Fix is written and tested on `audit/money-2026-08-03`, commit `8db58af`.
+
+---
+
+## 1. Credentials to rotate
+
+**None found — and that is a real result, not an absence of looking.**
+
+I searched all 107 commits on every branch for `sk_live`, `sk_test_51`, `pk_live_51`, `whsec_`,
+`BEGIN PRIVATE KEY`, `BEGIN RSA`, `BEGIN OPENSSH`, and for any commit ever *adding* a file matching
+`.env`, `*.pem`, `*.key`, `*.p12`, `*.pfx`, `*.sqlite`, `*.db`, or anything named like a dump or
+backup. The only `whsec_` hits in the entire history are three commits' worth of UI labels, a form
+placeholder and a validation regex. No credential has ever been committed to this repository.
+
+**So there is nothing to rotate from this repo's history.** The credentials that exist at runtime —
+Stripe secret and webhook keys, the Cloudflare tunnel token, the OpenMasjidOS per-app secret, the
+admin password hash — live only in the SQLite file on each masjid's data volume, or in the platform
+vault.
+
+Two related items that are *not* rotations but are worth your attention:
+
+- **Anyone with shell access to a masjid box can read the Stripe secret key.** It is stored in the
+  SQLite database with `0600` perms, but the container **runs as root** (DONATIONS-015) and the app
+  runs as root inside it, so the file perms protect against nothing that matters. Treat host access
+  to a masjid box as equivalent to holding their Stripe key.
+- **The Cloudflare tunnel token appears in the host process table** (DONATIONS-026), so any
+  unprivileged local user on the box can read it with `ps`. Fix is a one-line change to pass it via
+  `TUNNEL_TOKEN` in the child environment instead of argv — **I did not ship it because I cannot run
+  `cloudflared` here to confirm it reads that variable**, and getting it wrong silently breaks public
+  access. Worth doing next time someone can test a tunnel.
+
+---
+
+## 2. The git-history decision
+
+**No action needed.** History is clean (see §1). No `filter-repo`, no BFG, no force-push. I would
+have recommended against it anyway on a public repo with a live tag and a catalog pin.
+
+---
+
+## 3. Cross-repo changes needed in sibling repos
+
+Five sibling repos talk to this one and are being audited in parallel, so I implemented only the
+safe half here — validate, sanitise, log — and left the counterpart to you.
+
+### 3a. OpenMasjidOS — sanitise the email subject before it becomes an SMTP header
+**Relates to:** DONATIONS-023 (fixed on this side, commit `84bcae6`).
+This app now flattens CR/LF and exotic line separators out of the receipt subject before POSTing it
+to `/api/fabric/email`. But **any** app on the Fabric can send a subject, and the platform is what
+turns it into a real header. The platform must sanitise `subject` (and `to`) independently rather
+than trusting callers. I could not verify whether it currently does.
+
+### 3b. OpenMasjidOS — confirm the ingress sanitises `X-Forwarded-*`, then let apps use it
+**Relates to:** DONATIONS-009 (deferred here — see §4).
+`CLAUDE.md` §13 asserts the platform ingress sanitises these headers, but **no code in this repo
+reads them**, so the claim is currently untested. Behind the ingress every remote visitor shares one
+rate-limit bucket, which makes a trivial donation-DoS possible. To fix it safely this app needs a
+guarantee it can rely on. What I need from the platform side: a documented statement of which
+forwarded headers the ingress strips and rewrites, and ideally a header an app can trust as "this
+request came through the ingress" (a signed value, not a boolean anyone can send).
+
+### 3c. OpenMasjidAPPS — `docker-compose.yml` deviates from the catalog contract
+**Relates to:** DONATIONS-049. `CLAUDE.md` §10 requires the labels `com.openmasjid.app`,
+`com.openmasjid.service` and `com.openmasjid.managed`, and a platform-assigned port mapping
+(`"${OMOS_HOST_PORT_8080:-7870}:8080"`). This repo's compose has **none of the three labels** and
+hardcodes `"7870:8080"`. It evidently installs fine today, so either the spec or the platform's
+tolerance has drifted. Someone should decide which is authoritative and align them — I did not touch
+it, because compose is part of the published catalog contract.
+
+### 3d. OpenMasjidOS — what identity is allowed to become a Donations admin?
+**Relates to:** DONATIONS-051. `GET /api/session` mints a **full local admin session** for any
+identity the platform confirms; the username is recorded but never checked against a role. If
+OpenMasjidOS ever gains non-admin users, every one of them silently becomes a donations
+administrator with access to the Stripe keys and the donor ledger. Whether that is correct is a
+platform decision. If a role or scope claim exists (or should), this app should check it.
+
+---
+
+## 4. Decisions I need from you (deferred findings)
+
+### 4a. `/api/setup` during a platform outage (DONATIONS-005) — High
+Under SSO the local admin is never set, so `hasAdmin()` is false for ever, and the only guard on
+anonymous admin claiming is "is the platform reachable?". **During any platform outage, anyone who
+can reach the box can POST `/api/setup` and own the panel** — Stripe keys, donor ledger, everything.
+
+`CLAUDE.md` §13 documents this as the deliberate price of never bricking the panel, so I did not
+override it. Every obvious hardening breaks the escape hatch it exists for (a recovery code printed
+to the container log defeats a volunteer with no shell; restricting to private IPs doesn't help
+because a LAN attacker is already there; a boot-time window can be waited out).
+
+**My recommendation:** keep the hatch, make abuse loud. Fire a Fabric alert on every anonymous
+setup claim, and show "a local password was set on `<date>`" permanently in the panel until
+dismissed. Say the word and I'll implement it.
+
+### 4b. Rate limiters collapse behind the ingress (DONATIONS-009) — Medium
+See §3b. Needs the platform guarantee first. Symptom today: one attacker can exhaust the 30/min
+donation-intent budget for **all** remote donors, and lock out every remote admin login.
+
+### 4c. Anonymous donations are de-anonymised (DONATIONS-024) — Medium
+A donor who deliberately leaves name and email blank gets them **backfilled from Stripe's billing
+details** at confirm, so the cardholder name ends up in the ledger and the CSV. Is a blank name
+"I wish to be anonymous" or "I couldn't be bothered"? The masjid wants names for Gift Aid; the donor
+may have meant it. I won't guess — tell me which and I'll make it consistent everywhere.
+
+### 4d. Container runs as root (DONATIONS-015) — Medium
+Needs `USER node` **plus** an entrypoint that chowns `/data`, and **one real container start to
+prove the app can still write its database**. I have no Docker here. Shipping it unverified risks
+bricking every install on update. `docker-compose.yml`'s own comment already says this is pending
+CI validation.
+
+### 4e. Base images pinned by tag, not digest (DONATIONS-016) — Medium
+Same reason: a wrong digest fails the build, and the build workflow doesn't run on pull requests, so
+a mistake wouldn't surface until after merge. The change itself is mechanical once someone can run
+`docker build`.
+
+### 4f. No Content-Security-Policy (part of DONATIONS-014) — Medium
+I shipped `nosniff` and `no-referrer`, and deliberately **not** a CSP: Stripe's Payment Element
+loads `js.stripe.com` and its own frames, and a CSP that is even slightly wrong stops donors paying
+with no obvious error. It needs writing against a real Stripe Element in a browser. Highest-value
+missing header on the admin panel, and worth doing properly.
+
+### 4g. `@fastify/static` major upgrade (DONATIONS-040) — Low
+Four High advisories, all **refuted as exploitable in this configuration** (both registrations use
+`index: false`, never `list: true`, and both roots hold only already-public assets). The fix is
+8.3.0 → 10.1.2, a major bump across two majors. Left for a human on its own schedule.
+
+### 4h. Session revocation and password change (DONATIONS-013) — Medium
+There is no way to change the admin password and no way to invalidate a stolen 30-day cookie short
+of deleting the database. Needs a token-version scheme (bump a counter in the store, include it in
+the token, check it on verify) plus a change-password route. Straightforward, but it is an auth-model
+change with no existing auth-route tests, so it wants a human eye.
+
+---
+
+## 5. Assumptions I made
+
+State these back to me if any is wrong — several fixes rest on them.
+
+1. **Plain-HTTP LAN installs are still supported.** The whole design of the cookie `Secure` fix
+   (DONATIONS-012) is "follow the request scheme" rather than "always Secure", specifically so a
+   masjid on `http://box.local:7870` is not locked out. If HTTPS is now mandatory everywhere, the
+   simpler fix is `COOKIE_SECURE: "1"` in compose.
+2. **`x-forwarded-proto` may be read for the cookie flag** even though `trustProxy` is off, because
+   a spoofed value can only restrict the spoofer's own cookie. I am confident in this reasoning, but
+   it is reasoning, not a test.
+3. **Cloudflare may cache a `.csv` response** with no cache directives, since `.csv` is in its
+   default cached-extension list. The fix (`no-store`) is correct regardless, so this assumption
+   doesn't need to hold — but it is why I rated DONATIONS-003 High rather than Low.
+4. **Stripe's three-decimal set is exactly BHD, JOD, KWD, OMR, TND**, and those require a
+   multiple-of-10 minor amount. From Stripe's documented currency rules; I could not call the API to
+   confirm.
+5. **A one-minor-unit fixed fee is better than zero** for zero-decimal currencies (DONATIONS-008).
+   It is an approximation, not an FX conversion — the honest fix is an admin-visible per-account fee
+   model, which is a product decision.
+6. **`restart: unless-stopped` is in effect**, which is why `uncaughtException` now exits rather
+   than limping on (DONATIONS-029).
+7. **Nobody depends on `:latest`.** The catalog pins a commit and the compose pins a digest, so my
+   reading is that republishing `:latest` on a `main` push is untidy rather than dangerous. If
+   anything does track `:latest`, the push veto matters even more than I've described.