Server security review — remediation

Response to the static security review of apps/server (21 findings: 7 high, 11 medium, 3 low). This page tracks what was fixed in this repo versus what is owned by infra / DevOps (DEV-415) or still needs a product decision.

Implementation lives under apps/server/src/; the new configuration knobs are listed under Configuration knobs and in .env.example.

Fixed in-repo

#	Severity	Finding	Fix
F3	high	JSON body-limit bypassed by a non-JSON Content-Type	The limiter bounds every body regardless of Content-Type. Only POST /api/files/upload and POST /admin/channel/media skip it; those routes apply their own larger multipart cap. middleware/bodyLimits.ts
F7	high	Runtime identity is also the migration identity	SKIP_MIGRATIONS=1 lets migrations run as a separate deploy job under a short-lived role, so the runtime role can be CRUD-only. index.ts
F4	high	DB TLS not enforced	REQUIRE_DB_VERIFY_FULL=1 asserts sslmode=verify-full at boot (opt-in; infra enforces too). env.ts
F6	high	Arbitrary/plaintext S3 endpoint	REQUIRE_S3_HTTPS=1 asserts an https S3 endpoint at boot (opt-in). env.ts
F8	medium	Super-admin password passed in argv	admin:create reads the password from stdin / a no-echo TTY prompt, never argv. db/createAdmin.ts
F9	medium	User JWTs have no verifier-side max age	JWT_MAX_AGE caps accepted token age independent of exp. middleware/auth.ts
F11	medium	Abuse-control maps grow unbounded	Hard-cap the login-failure map. Past the cap, drop expired keys, then the oldest keys that are not a live lock, so a flood cannot clear an active account or IP lockout. middleware/adminAuth.ts
F13	medium	A user-manager could remove a higher-privileged admin	Deactivate, delete, and rewriting permissions require dominance over the target's current set, so a user-manager cannot shrink a stronger admin and then remove them. routes/adminUsers.ts
F14	medium	Concurrent comment reports bypassed dedup	Partial unique index on (comment_id, reporter_user_id) WHERE status='open' + ON CONFLICT DO NOTHING (migration 0026).
F15	medium	Admin lockout sustainable indefinitely	The fail count resets once a lock window lapses, so re-locking needs a fresh burst. middleware/adminAuth.ts
F16	medium	Concurrent reaction toggles corrupt the tally	pg_advisory_xact_lock serializes per (post, user) inside the transaction. routes/channel.ts
F17	medium	Devtools bulk reset gated only by a flag	Also requires an out-of-band ops token (X-Devtools-Token = DEVTOOLS_RESET_TOKEN). routes/devtools.ts
F18	medium	Stored media URLs could force client fetch to any host	isSafeMediaUrl rejects loopback/private/link-local/reserved hosts + userinfo; applied to avatars, rich-text images, and Intercom attachments. Optional REQUIRE_MEDIA_HTTPS / SAFE_MEDIA_HOSTS.
F19	low	Conversation/ticket existence oracle (404 vs 403)	Foreign objects return 404, indistinguishable from missing. routes/conversations.ts, routes/tickets.ts
F20	low	Duplicate CSAT repeated Intercom note/tag	The Intercom side effects only fire on a first (non-duplicate) insert. routes/csat.ts
F21	low	Admin username timing oracle	Unknown/inactive usernames pay an equal dummy scrypt. middleware/adminAuth.ts, services/password.ts

Partially fixed (app side done; rest owned elsewhere)

#	Severity	App-side fix	Remaining
F5	high	Structured, secret-free admin auth events (login success/failure/lockout/logout) → stdout + durable admin_logs. services/adminAudit.ts	Ship stdout to the external append-only SIEM (infra, DEV-415 item 8).
F10	medium	A Range redirect's presigned URL is capped to the presenting token's remaining lifetime. services/s3.ts, routes/publicFiles.ts	Revoke a link when its content is hidden/deleted — needs a per-request status check and a product call on hide reversibility.
F12	medium	Admin media is validated before it is stored, so a rejected upload leaves no orphan. routes/adminChannel.ts	Cumulative per-user upload quota + abandoned-object lifecycle (a quota store + bucket lifecycle = infra).

Owned by infra / DevOps (DEV-415)

#	Severity	Finding	Owner
F1	high	Mutable, script-capable container build; runtime inherits the build stage	Digest-pin images + verify lockfile + minimal runtime stage (DEV-415 item 5).
F2	high	Admin plane is password-only on the public listener	Corporate IdP + mandatory MFA + network isolation in front of /admin (DEV-415 item 1).

Configuration knobs

All opt-in — defaults preserve current behaviour, so enabling them can't surprise a not-yet-ready deployment. See apps/server/.env.example.

Env var	Effect
JWT_MAX_AGE	Verifier-side maximum token age, e.g. 1h (F9).
SKIP_MIGRATIONS	1 → the runtime process does not migrate; a separate job does (F7).
REQUIRE_DB_VERIFY_FULL	1 → production requires sslmode=verify-full (F4).
REQUIRE_S3_HTTPS	1 → production requires an https S3 endpoint (F6).
REQUIRE_MEDIA_HTTPS	1 → reject plaintext-http media URLs (F18).
SAFE_MEDIA_HOSTS	Comma/space list — allowlist media hosts (+ subdomains) (F18).
DEVTOOLS_RESET_TOKEN	Ops token required for the devtools bulk reset (F17).

Verification

Each change is covered by the server test suite (pnpm --filter @aw-chat/server test, needs a local Postgres); new behaviour added unit tests for F3, F9, and F18, and the conversation/ticket isolation tests assert the F19 404. See also the earlier SEC-48 handoff for the SEC-72/76/84 work.