Skip to content

audit: security + code-health fixes (13 findings, verified) - #2

Merged
osayed0001 merged 15 commits into
mainfrom
audit/security-2026-08-03
Aug 4, 2026
Merged

audit: security + code-health fixes (13 findings, verified)#2
osayed0001 merged 15 commits into
mainfrom
audit/security-2026-08-03

Conversation

@hasan-ismail

Copy link
Copy Markdown
Member

Security and code-health audit of 2026-08-03. 13 fixes, all verified. Full report in
docs/audit/SECURITY_AUDIT.md; what shipped and how it was checked
in docs/audit/REMEDIATION.md.

Why this is a PR and not a push to main

A push to main triggers build-image.yml, which republishes
ghcr.io/openmasjid-solutions/openmasjiddonations:0.38.0 and :latest — the live production tag
the App Store catalog's digest pin resolves to. That is a published artifact, so autonomous push was
disabled and everything came here instead.

Highest-value fix

DONATIONS-003 (High) — the donor-PII CSV export had no Cache-Control at a .csv URL.
.csv is in Cloudflare's default cached-extension list, so on a publicly-tunnelled install the edge
could have served every donor's name and email to a request carrying no session cookie.

Behaviour changes to watch (Tier 2)

  • Session cookie is now Secure on HTTPS. It follows the request scheme, so plain-HTTP LAN
    installs are unaffected — that was the design constraint, since always-Secure locks a masjid out
    of its own panel. Revert 70d5457 first if HTTPS admins report sign-in trouble.
  • New audit_log table (additive, CREATE TABLE IF NOT EXISTS, reverse migration in
    REMEDIATION.md) plus the admin's username now carried inside the signed session token.
  • Three new rate limits and two new response headers on every route.
  • unhandledRejection no longer kills the process — it logs and keeps serving.

Verification

130 → 171 tests, 0 failures. Typecheck, build and web build clean on every commit, not just at the
end. Server npm audit 4 high → 1 high (the remaining one is a two-major @fastify/static bump that
the adversarial pass refuted as exploitable in this configuration); web 1 high → 0.

Two regression tests were proven to fail against the pre-fix code and pass after — the login-limiter
sweep (2 failures before) and the email header injection (1 before). Real output is pasted in
REMEDIATION.md.

Two defects my own new tests caught in my own fixes are recorded there too, because they are the
argument for having written them: an audit-log ordering bug (same-millisecond rows tie-broke on
random hex) and a connectionTimeout that would have reaped healthy keep-alive sockets.

One thing to do before merging

The Action SHA pins (73086c5) were verified by API but not executedbuild-image.yml only
runs on main, v* tags and workflow_dispatch, so a PR never exercises it. Please
workflow_dispatch it once on this branch first.

Not fixed, deliberately

7 findings need a decision or a runtime I don't have (non-root container, base-image digests, CSP,
the /api/setup outage window, the ingress rate-limit bucket, anonymous-donation policy, tunnel
token via env). All in docs/audit/ACTION_REQUIRED.md with my
recommendation for each. Nothing was marked fixed that I could not verify.

…ripe webhook, and monthly intents [DONATIONS-019, DONATIONS-020, DONATIONS-010]
…equestTimeout [DONATIONS-018, DONATIONS-021]
…e, lock the data dir to 0700 [DONATIONS-041, DONATIONS-042, DONATIONS-027, DONATIONS-028]
…promise rejection [DONATIONS-043, DONATIONS-029]
…selves [DONATIONS-052]

Also corrects CLAUDE.md §11, which specified argon2 for the admin password where the
implementation has always used scrypt [DONATIONS-046].
@osayed0001
osayed0001 merged commit 46516a0 into main Aug 4, 2026
2 checks passed
@hasan-ismail
hasan-ismail deleted the audit/security-2026-08-03 branch August 5, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants