Fixing security vulnerabilities, adding a docker compose file, and fixing app launch as it did not launch before due to package lock not lining up with what the application was requiring.

[SluberskiHomeLab]

Summary

Full security review of FormBase, and remediation of 19 of 21 findings — including a critical authentication bypass that allowed full account takeover of any user.

Every finding was confirmed by exploiting it against a running instance before being fixed, and each has a regression test that fails on the vulnerable code and passes after.

	Result
Security suite (baseline, vulnerable code)	6 passed / 33 failed
Security suite (host)	40 passed / 0 failed
Security suite (Docker)	40 passed / 0 failed
Functional smoke suite	21 passed / 0 failed (host + Docker)
npm audit	0 vulnerabilities (was 1 high)

Scope note: the Docker build fix (lockfile drift, Express 5 migration, multi-stage image) is already on master. This PR is the security work that followed from reviewing that code, and contains no build changes beyond USER node and the nodemailer bump.

Findings

ID	Severity	Finding	Status
S1	🔴 CRITICAL	Hardcoded fallback JWT signing secret → account takeover	Fixed
S2	🟠 HIGH	SSRF via user-controlled webhook_url	Fixed
S3	🟠 HIGH	No rate limiting anywhere, including login	Fixed
S4	🟡 MEDIUM	Origin allowlist bypassable by substring match	Fixed
S5	🟡 MEDIUM	CSV formula injection in export	Fixed
S6	🟡 MEDIUM	Header injection via Content-Disposition filename	Fixed
S7	🟡 MEDIUM	Unescaped user input in notification email HTML	Fixed
S8	🟡 MEDIUM	Unescaped sinks in dashboard SPA (incl. latent stored XSS)	Fixed
S9	🔵 LOW	Content-Security-Policy explicitly disabled	Fixed
S10	🔵 LOW	CORS wide open	Fixed
S11	🔵 LOW	Internal error messages returned to clients	Fixed
S12	🔵 LOW	Unauthenticated /api/health leaked instance stats	Fixed
S13	🔵 LOW	Open redirect via redirect_url	Fixed
S14	🔵 LOW	Login timing revealed which accounts exist	Fixed
S15	🔵 LOW	No server-side email validation; unbounded password	Fixed
S16	🔵 LOW	30-day JWTs with no revocation	⚠️ Partial
S17	⚪ INFO	Container ran as root	Fixed
S18	⚪ INFO	nodemailer advisory GHSA-p6gq-j5cr-w38f	Fixed
S19	⚪ INFO	apiKeyMiddleware was dead code	Fixed
S20	⚪ INFO	JWT stored in localStorage	⚠️ Accepted
S21	⚪ INFO	Dashboard "Export CSV" button was broken (401)	Fixed

The three that matter most

S1 — Hardcoded JWT secret (CRITICAL)

src/auth.js fell back to 'formbase-dev-secret-change-in-production', a literal published in this repository. Any instance that did not set JWT_SECRET signed tokens that anyone could forge.

Exploited: a token signed offline with that string was accepted by /api/auth/me (HTTP 200) — full account takeover given only a user id. The readme listed JWT_SECRET as optional with the default shown.

Fixed in src/lib/secret.js. No constant fallback exists. JWT_SECRET is used when ≥32 chars; otherwise a 48-byte random secret is generated once into ${DATA_DIR}/.jwt-secret (mode 0600) and reused across restarts. The old literal and other known-weak values are rejected at boot rather than silently accepted.

Verified: forged token → 401 · boot with the old literal refuses to start · boot with a 5-char secret refuses to start · token issued before docker restart still valid after · file mode confirmed -rw-------.

S2 — SSRF via webhook_url (HIGH)

Set through the authenticated form API with no validation, then fetched server-side on every submission.

Exploited: a form pointed at http://127.0.0.1:3121/internal-admin-endpoint made the server POST submission JSON to that loopback listener. On a cloud host this reaches 169.254.169.254 and anything else routable from the container.

Fixed in src/lib/safe-url.js. Scheme restricted to http/https on write; before each send the hostname is resolved and every returned address checked against loopback, RFC1918, CGNAT, link-local, ULA, multicast and reserved ranges. redirect: 'manual' so a 302 cannot bounce into private space, plus a 5s timeout. SSRF_ALLOW_PRIVATE=1 for genuine LAN webhooks.

Verified: loopback, link-local and file:// all rejected 400 — and with a private URL written directly into SQLite, bypassing the API entirely, the listener still received nothing.

S3 — No rate limiting (HIGH)

rate-limiter-flexible was a declared dependency that was never imported anywhere. The intent was there; the wiring was not.

Exploited: 25 failed logins in 1268 ms, zero 429s.

Fixed in src/lib/rate-limit.js, wiring up the dependency already being shipped in every image. Auth 10/IP/15min and 5/account/15min; submissions 30/IP/form/min; general API 300/IP/min. All budgets env-tunable via RATE_LIMIT_*. Successful auth refunds the bucket; 429s carry Retry-After.

Verified against shipped defaults: login throttled after 6 attempts with Retry-After: 900; submissions throttled after 31.

One finding only existed because of another fix

Worth calling out for reviewers, because neither change is dangerous alone.

s.ip was rendered unescaped in the dashboard. That is harmless while req.ip is the socket address — but rate limiting (S3) needs trust proxy, and enabling that makes X-Forwarded-For attacker-controlled, turning s.ip into stored XSS in the owner's dashboard, triggered by an unauthenticated submission.

They ship together: trust proxy is opt-in via TRUST_PROXY, the server stores only a net.isIP()-validated address, and every dashboard sink is escaped.

Verified in a real browser: created a form carrying <img src=x onerror=...> in all seven previously-unescaped sinks, plus a submission with payloads in the key, the value, and X-Forwarded-For. Result: 0 of 7 XSS flags fired, 0 injected <img> tags, payloads rendered as literal text.

CSP was fixed properly, not with 'unsafe-inline'

A policy permitting 'unsafe-inline' would have been theatre. Instead the 14.6 KB inline <script> moved out of index.html into public/app.js, and every inline onclick= was replaced with delegated data-nav / data-action handlers — so the policy can genuinely be script-src 'self'.

Verified in-browser that the policy is enforced, not merely present: an injected inline <script> did not execute, and an external CDN script was blocked.

Reviewing this

15 files, +1465 / −397. Suggested order:

1. src/lib/secret.js (+57) — the critical fix. Start here.
2. src/auth.js — secret wiring, 7d expiry, X-API-Key support, constant-time key compare.
3. src/lib/safe-url.js (+127) — SSRF guards. The blocked-CIDR table is the security-critical part.
4. src/routes/submit.js — SSRF call site, origin comparison, email escaping, IP validation.
5. src/lib/rate-limit.js (+89) and src/lib/escape.js (+37) — small, self-contained.
6. src/routes/forms.js, src/routes/users.js, src/server.js — call sites and app wiring.
7. test/security.js (+450) — one test per finding; useful as a spec for what each fix claims.

⚠️ public/app.js (+301) and public/index.html (−266) are mostly a file move, not new logic. Reviewing them as a rename with esc() added at seven sinks is far faster than reading 300 new lines. git diff -M or a whitespace-blind diff helps.

Testing

npm ci
npm start
npm run test:security -- http://localhost:3000

The rate-limit tests (S3) need an instance running shipped default budgets, while the rest of the suite needs headroom so it isn't throttled by its own traffic. Run two instances and pass both:

RATE_LIMIT_AUTH_IP=5000 RATE_LIMIT_AUTH_ACCOUNT=5000 RATE_LIMIT_SUBMIT=5000 PORT=3030 DATA_DIR=/tmp/fb-loose npm start
PORT=3031 DATA_DIR=/tmp/fb-strict npm start
node test/security.js http://localhost:3030 http://localhost:3031

Set FORMBASE_DATA_DIR to a locally-readable data dir to additionally exercise S2d, which writes a private webhook URL straight into SQLite to prove the fetch-time guard holds independently of API validation.

⚠️ Deployment notes — behaviour changes on upgrade

• All existing tokens are invalidated on instances that relied on the default JWT secret; users must log in again. This is the point of S1.
• JWT_SECRET, if set, must now be ≥32 characters and must not be the old published literal, or the process refuses to start.
• Token lifetime 30d → 7d (JWT_EXPIRES_IN to override).
• /api/health no longer returns counts — update monitoring that parsed them; use authenticated /api/health/stats.
• /api is same-origin only unless CORS_ORIGIN is set. /f stays cross-origin by design.
• webhook_url pointing at private or loopback addresses is now rejected. Set SSRF_ALLOW_PRIVATE=1 if your webhook target is genuinely on the LAN.
• redirect_url must be http or https.
• Set TRUST_PROXY when running behind a reverse proxy, or rate limiting keys off the proxy's address. Unset is the safe default.
• Rate limits may be tight where many users share a source IP — tune with RATE_LIMIT_AUTH_IP / RATE_LIMIT_AUTH_ACCOUNT / RATE_LIMIT_SUBMIT / RATE_LIMIT_API.
• Bind-mounted data directories must be chowned to uid 1000 now that the container runs as non-root. Named volumes inherit ownership automatically.
• Emails are normalised to lowercase on write; lookups use COLLATE NOCASE so accounts created before this change can still log in.

Not fixed

• S16 — token revocation (partial). Lifetime cut 30d → 7d, but true revocation needs a token-version column checked per request. Regenerating an API key still does not invalidate outstanding JWTs.
• S20 — JWT in localStorage (accepted). Moving to httpOnly cookies would trade XSS exfiltration for CSRF surface and a significantly larger refactor. Materially reduced by the CSP in S9.
• User enumeration on registration remains by design — a duplicate registration still returns 409. Removing it needs an email-verification flow. It is now rate limited.
• DNS rebinding against the SSRF guard is narrowed but not eliminated: addresses are validated after resolution, but a TOCTOU window remains between the check and the connection. Closing it fully requires pinning the socket to the validated IP.

🤖 Generated with Claude Code

[SluberskiHomeLab]

This is from my fork, thought you might want to see what I did to it to make it function and to solve some critical vulnerabilities.

[Copilot]

Pull request overview

This pull request hardens FormBase against a wide range of security issues identified in a security review (auth hardening, SSRF defenses, rate limiting, output escaping/XSS defenses, safer CSV export, and safer defaults), and adds Docker Compose–based deployment docs/config for easier self-hosting.

Changes:

• Reworked authentication and request handling to remove unsafe JWT secret fallback, add API-key auth wiring, reduce JWT lifetime, and prevent internal error leakage.
• Added SSRF protections for webhooks/redirects, added rate limiting, and added output escaping/CSV export hardening (CSP, XSS and CSV injection defenses).
• Added Dockerfile + docker-compose.yml + .env.example and a security regression test suite (test/security.js) to validate fixes against a running instance.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/security.js	Adds a live-instance security regression suite (one test per finding) to prevent regressions.
src/server.js	Centralizes security posture: CSP via Helmet, request size limits, /api routing behavior, error handler, and health endpoint changes.
src/routes/users.js	Adds credential validation, rate limiting integration, email normalization, and timing-equalized auth flow.
src/routes/submit.js	Adds SSRF-safe webhook delivery, strict origin allowlist matching, email escaping, IP validation, and submission rate limiting.
src/routes/forms.js	Adds URL scheme validation for stored URLs, CSV export hardening, safer pagination coercion, and API-key-capable auth middleware.
src/lib/secret.js	Implements safe JWT secret resolution (reject known-bad values; persist generated secret to disk with restrictive permissions).
src/lib/safe-url.js	Introduces URL validation + DNS-resolution-based private-address blocking to mitigate SSRF.
src/lib/rate-limit.js	Wires rate-limiter-flexible into auth, submissions, and general API limiting.
src/lib/escape.js	Adds HTML escaping, CSV cell neutralization, and safe Content-Disposition generation helpers.
src/auth.js	Removes hardcoded JWT secret fallback, adds API-key auth support, constant-time key comparison, and configurable JWT expiry.
readme.md	Updates Node version guidance and documents Docker Compose recommended setup and key security-related env vars.
public/index.html	Removes large inline dashboard script to support strict CSP; loads public/app.js.
public/app.js	New extracted dashboard SPA script with escaping and non-inline event handlers to comply with CSP and reduce XSS risk.
package.json	Updates engines requirement, bumps dependencies, and adds test:security script.
package-lock.json	Updates lockfile to reflect dependency/version changes (including license and engine metadata).
Dockerfile	Adds multi-stage build and runs container as non-root with persistent data directory ownership handling.
dockerfile	Removes old lowercase dockerfile in favor of the new Dockerfile.
docker-compose.yml	Adds a Compose setup with persistent named volume, healthcheck, and env passthrough for configuration.
.env.example	Documents all supported runtime configuration values for Compose-based deployments.

Comments suppressed due to low confidence (2)

src/lib/rate-limit.js:86

• clearAuthAttempts() deletes the entire per-IP bucket on any successful auth/register. That allows an attacker to reset the IP limiter by successfully authenticating to any account, then continue guessing passwords against other accounts from the same IP. Consider only clearing the per-account limiter (or rewarding points) rather than deleting the IP bucket.

async function clearAuthAttempts(req) {
  const account = String(req.body?.email || '').toLowerCase().slice(0, 254);
  try {
    await limiters.authIp.delete(clientKey(req));
    if (account) await limiters.authAccount.delete(account);
  } catch { /* best effort */ }

src/lib/rate-limit.js:73

• Per-account rate limiting keys off req.body.email without trimming whitespace. Login normalizes with trim().toLowerCase(), so an attacker can bypass the account bucket by adding trailing/leading spaces to the email while still hitting the same account in the DB lookup.

  return async (req, res, next) => {
    const account = String(req.body?.email || '').toLowerCase().slice(0, 254);
    try {
      await limiters.authIp.consume(clientKey(req));
      if (account) await limiters.authAccount.consume(account);
      next();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.