ghostguild-org/docs/SECURITY_EVALUATION.md
Jennie Robinson Faber 29c96a207e Add Vitest security test suite and update security evaluation doc
Set up Vitest with server (node) and client (jsdom) test projects.
79 tests across 8 files verify all Phase 0-1 security controls:
escapeHtml sanitization, DOMPurify markdown XSS prevention, CSRF
enforcement, security headers, rate limiting, auth guards, profile
field allowlist, and login anti-enumeration. Updated SECURITY_EVALUATION.md
with remediation status, implementation summary, and automated test
coverage details.
2026-03-01 12:30:06 +00:00

282 lines
14 KiB
Markdown

# Ghost Guild Security Evaluation
**Date:** 2026-02-28 (updated 2026-03-01)
**Framework:** OWASP ASVS v4.0, Level 1
**Scope:** Full application stack (Nuxt 4 + Nitro server + MongoDB)
---
## Framework Rationale
Ghost Guild handles payments (Helcim), PII, and user-generated content. We evaluated four frameworks before selecting ASVS:
| Framework | Why Not |
|---|---|
| PCI-DSS | Too narrow -- only covers payment surface, misses auth/XSS/access control |
| NIST CSF / ISO 27001 | Organizational frameworks, not application-level |
| OWASP Top 10 | Too coarse -- categories without testable requirements |
| **OWASP ASVS L1** | **Selected** -- purpose-built for web apps, testable pass/fail criteria across 14 chapters |
ASVS Level 1 targets "all software" and is achievable for a small team.
---
## Findings
### Severity Summary
| Severity | Count | Key Areas |
|----------|-------|-----------|
| CRITICAL | 5 | Admin auth disabled, payment verification stub, XSS (markdown + emails), cookie flags |
| HIGH | 9 | No rate limiting, no CSRF, JWT secret fallback, unauthenticated endpoints, mass assignment |
| MEDIUM | 5 | Long-lived tokens, Slack secrets unused, devtools, data logging, regex injection |
| LOW | 2 | User enumeration, email format validation |
---
### CRITICAL
#### C1. Admin endpoints completely unprotected
- **ASVS:** V4.1.1 (Trusted enforcement)
- **Files:** All routes under `server/api/admin/`
- **Evidence:** Auth code is commented out with `// TODO: Temporarily disabled auth for testing`. Anyone can list all members, create/delete events, and view revenue dashboard. The Member model has no `role` or `isAdmin` field.
#### C2. Payment verification is a stub
- **ASVS:** V10.2.1 (Business logic integrity)
- **File:** `server/api/helcim/verify-payment.post.js`
- **Evidence:** Returns `{ success: true, cardToken: body.cardToken }` without calling the Helcim API.
#### C3. XSS via unsanitized markdown
- **ASVS:** V5.3.3 (Output encoding for HTML)
- **Files:** `app/composables/useMarkdown.js`, `app/pages/members.vue:247`
- **Evidence:** `marked()` output rendered via `v-html` with no DOMPurify.
#### C4. XSS in email templates
- **ASVS:** V5.2.6 (Server-side injection)
- **File:** `server/utils/resend.js` (11+ interpolation points)
- **Evidence:** User-supplied values interpolated directly into HTML email bodies.
#### C5. Session cookie allows JavaScript access
- **ASVS:** V3.2.1 (Cookie attributes)
- **File:** `server/api/auth/verify.get.js:40-45`
- **Evidence:** `httpOnly: false, secure: false`. Session token accessible to any JavaScript.
---
### HIGH
#### H1. No rate limiting on any endpoint
- **ASVS:** V13.2.5
- **Evidence:** No rate limiting middleware anywhere. Login, registration, uploads, and payment endpoints are unlimited.
#### H2. No CSRF protection
- **ASVS:** V3.5.2
- **Evidence:** No CSRF tokens, double-submit cookies, or CSRF middleware.
#### H3. Hardcoded JWT fallback secret
- **ASVS:** V6.4.1 (Key management)
- **File:** `nuxt.config.ts:17`
- **Evidence:** `jwtSecret: process.env.JWT_SECRET || 'dev-secret-change-in-production'`
#### H4. File upload endpoint unauthenticated
- **ASVS:** V4.1.3
- **File:** `server/api/upload/image.post.js`
- **Evidence:** No auth check. Anyone can upload images.
#### H5. Helcim payment endpoints mostly unauthenticated
- **ASVS:** V4.1.1
- **Files:** `verify-payment.post.js`, `initialize-payment.post.js`, `update-billing.post.js`, `subscription.post.js`
#### H6. Mass assignment: helcimCustomerId in allowedFields
- **ASVS:** V5.1.3
- **File:** `server/api/members/profile.patch.js:40`
- **Evidence:** A member can change their own Helcim customer ID.
#### H7. No input validation
- **ASVS:** V5.1.3
- **Evidence:** Zod is installed but has zero imports. All validation is ad-hoc.
#### H8. User enumeration on login
- **ASVS:** V2.1.7
- **File:** `server/api/auth/login.post.js:24-28`
- **Evidence:** Returns 404 "No account found" vs success.
#### H9. No security headers
- **ASVS:** V9.1.1
- **Evidence:** No CSP, HSTS, X-Frame-Options, or X-Content-Type-Options.
---
### MEDIUM
| ID | ASVS | Finding | File |
|----|------|---------|------|
| M1 | V2.5.2 | 30-day static session token, no rotation | `verify.get.js:36` |
| M2 | V10.2.2 | Slack signing secret defined but never used | `nuxt.config.ts:21` |
| M3 | V7.4.1 | DevTools enabled unconditionally | `nuxt.config.ts:4` |
| M4 | V7.1.1 | Console logging of payment data and API token substrings | `helcim/customer.post.js:42` |
| M5 | V5.3.4 | Unescaped regex in directory search | `members/directory.get.js:49-51` |
### LOW
| ID | ASVS | Finding | File |
|----|------|---------|------|
| L1 | V2.1.7 | Timing-based enumeration via DB lookup | `auth/login.post.js` |
| L2 | V5.1.3 | No email format validation on login | `auth/login.post.js:15` |
---
## Future Feature Risk Assessment
| Planned Feature | Risk | Must Address First |
|---|---|---|
| Rich text member updates | Same XSS pattern as C3 | Fix markdown sanitization (C3) |
| Resource library with downloads | Unauthenticated upload (H4), malware distribution | Add upload auth (H4), file validation |
| Etherpad integration | External content rendered unsanitized | Build sanitization utility (C3) |
| Cal.com integration | API credential exposure | Fix secret management (H3) |
| Member-proposed events | No admin role model, no approval workflow | Build RBAC (C1) |
| Advanced search/analytics | Regex injection (M5), privacy leakage | Fix regex escaping (M5) |
---
## Remediation Summary (Phases 0-1 + partial Phase 2)
All work lives on branch `security/asvs-remediation`.
### Auth guards (`server/utils/auth.js`)
- `requireAuth(event)` -- Reads JWT from `auth-token` cookie, verifies against `jwtSecret`, loads member from DB, rejects suspended/cancelled accounts (403). Auto-imported by Nitro.
- `requireAdmin(event)` -- Calls `requireAuth`, then checks `member.role === 'admin'` (403 if not). Member model gained `role` field (enum: `member`/`admin`, default `member`).
- Applied to all `server/api/admin/`, `server/api/upload/`, and `server/api/helcim/` endpoints.
### CSRF (`server/middleware/01.csrf.js` + `app/plugins/csrf.client.js`)
- Double-submit cookie pattern. Middleware generates a random token cookie on first request, then enforces that POST/PATCH/DELETE to `/api/*` include a matching `x-csrf-token` header.
- Client plugin reads the cookie and attaches the header on every `$fetch` request.
- Exempt routes: `/api/helcim/webhook`, `/api/slack/webhook`, `/api/auth/verify`.
### Security headers (`server/middleware/02.security-headers.js`)
- Always: `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `X-XSS-Protection: 0`, `Referrer-Policy: strict-origin-when-cross-origin`, `Permissions-Policy: camera=(), microphone=(), geolocation=()`.
- Production only: HSTS (1 year, includeSubDomains) and CSP allowing Helcim, Cloudinary, and Plausible sources.
### Rate limiting (`server/middleware/03.rate-limit.js`)
- `rate-limiter-flexible` with in-memory stores, keyed by client IP.
- Auth endpoints: 5 requests per 5 minutes. Payment endpoints: 10 per minute. Uploads: 10 per minute. General API: 100 per minute.
- Returns 429 with `Retry-After` header on exhaustion.
### XSS prevention
- **Markdown** (`app/composables/useMarkdown.js`): `marked()` output sanitized through DOMPurify with explicit allowlists for tags and attributes. Strips script/iframe/object/embed/img and all event handler attributes.
- **Email templates** (`server/utils/escapeHtml.js`): Pure function escaping `& < > " '` to HTML entities. Applied to all user-supplied interpolations in `server/utils/resend.js`.
### Anti-enumeration (`server/api/auth/login.post.js`)
- Login returns identical `{ success: true, message: "If this email is registered, we've sent a login link." }` for both existing and non-existing emails.
### Mass assignment (`server/api/members/profile.patch.js`)
- Explicit allowlist of profile fields. `helcimCustomerId`, `role`, `status`, `email`, `_id` are excluded from the `$set` update.
### Session management
- 7-day token expiry with refresh endpoint at `/api/auth/refresh`.
---
## Remediation Roadmap
### Phase 0: Emergency (before any production traffic) -- COMPLETE
| # | Finding | Fix | ASVS | Status |
|---|---------|-----|------|--------|
| 1 | C5 | Set `httpOnly: true`, `secure` conditional on NODE_ENV | V3.2.1 | Done |
| 2 | C3 | Install `isomorphic-dompurify`, sanitize `marked()` output | V5.3.3 | Done |
| 3 | C1 | Add `role` field to Member model, re-enable admin auth with role check | V4.1.1 | Done |
| 4 | C2 | Call Helcim API in verify-payment to confirm transactions server-side | V10.2.1 | Done |
| 5 | H3 | Throw on missing JWT_SECRET instead of fallback | V6.4.1 | Done |
### Phase 1: Pre-launch (before public access) -- COMPLETE
| # | Finding | Fix | ASVS | Status |
|---|---------|-----|------|--------|
| 6 | C4 | Create `escapeHtml()` utility, apply to all email template interpolations | V5.2.6 | Done |
| 7 | H4 | Add JWT verification to upload endpoint | V4.1.3 | Done |
| 8 | H5 | Add JWT verification to payment endpoints | V4.1.1 | Done |
| 9 | H6 | Remove `helcimCustomerId` from `allowedFields` | V5.1.3 | Done |
| 10 | H2 | Add CSRF double-submit cookie middleware (exempt webhooks) | V3.5.2 | Done |
| 11 | H9 | Add CSP, HSTS, X-Frame-Options, X-Content-Type-Options headers | V9.1.1 | Done |
| 12 | H1 | Add rate limiting to login, registration, upload, payment | V13.2.5 | Done |
| 13 | H8 | Return identical response for existing/non-existing accounts | V2.1.7 | Done |
| 14 | -- | Add `status: 'active'` check to auth endpoints | V4.1.1 | Done |
### Phase 2: Hardening (within 30 days of launch) -- PARTIAL
| # | Finding | Fix | ASVS | Status |
|---|---------|-----|------|--------|
| 15 | H7 | Implement Zod validation across all API endpoints | V5.1.3 | Open |
| 16 | M5 | Escape regex in directory search | V5.3.4 | Open |
| 17 | M4 | Remove sensitive console.log statements | V7.1.1 | Open |
| 18 | M3 | Make devtools conditional on NODE_ENV | V7.4.1 | Open |
| 19 | M1 | Shorter session tokens (7d) with refresh endpoint | V2.5.2 | Done |
| 20 | -- | Create shared `requireAuth()`/`requireAdmin()` utilities | V4.1.1 | Done |
### Phase 3: Before building planned features
| # | Fix | Status |
|---|-----|--------|
| 21 | Build sanitization utility (DOMPurify wrapper) for all user-generated HTML | Open |
| 22 | Design admin role model with granular permissions | Open |
| 23 | Implement file validation pipeline (type, size, virus scanning) | Open |
| 24 | Design credential management patterns (encrypted at rest) | Open |
---
## Automated Testing
### Framework
Vitest with two test projects:
- **Server tests** (`tests/server/`): Node.js environment, h3 globals stubbed from real h3 functions
- **Client tests** (`tests/client/`): jsdom environment for browser-side composables
```bash
npm run test # Watch mode
npm run test:run # Single run (CI)
```
### Infrastructure
- `tests/server/setup.js` -- Stubs real h3 functions (`getCookie`, `setCookie`, `getMethod`, `getHeader`, `setHeader`, `getRequestURL`, `createError`, `defineEventHandler`, `readBody`, etc.) as globals to simulate Nitro auto-imports. Also stubs `useRuntimeConfig`.
- `tests/server/helpers/createMockEvent.js` -- Factory that builds real h3 events from Node.js `IncomingMessage`/`ServerResponse` pairs. Accepts `method`, `path`, `headers`, `cookies`, `body`, and `remoteAddress`. Captures response headers via `event._testSetHeaders` for assertions.
### Test Coverage (79 tests across 8 files)
| File | Tests | Security Controls Verified |
|------|-------|---------------------------|
| `tests/server/utils/escapeHtml.test.js` | 12 | All 5 HTML entity escapes, null/undefined handling, `<script>` and `<img onerror>` XSS payloads (C4, V5.2.6) |
| `tests/client/composables/useMarkdown.test.js` | 18 | Script/iframe/object/embed tags stripped, onerror/onclick attrs stripped, javascript: URIs sanitized, safe markdown preserved (C3, V5.3.3) |
| `tests/server/middleware/csrf.test.js` | 14 | GET/HEAD/OPTIONS bypass, non-API bypass, webhook exemptions, 403 on missing/mismatched token, PATCH/DELETE enforcement, cookie provisioning (H2, V3.5.2) |
| `tests/server/middleware/security-headers.test.js` | 12 | X-Content-Type-Options, X-Frame-Options, X-XSS-Protection, Referrer-Policy, Permissions-Policy always set; HSTS + CSP production-only; CSP includes Helcim/Cloudinary/Plausible sources (H9, V9.1.1) |
| `tests/server/middleware/rate-limit.test.js` | 4 | Auth endpoint 5/5min limit, payment endpoint 10/min limit, IP isolation between clients (H1, V13.2.5) |
| `tests/server/utils/auth.test.js` | 8 | No cookie = 401, invalid JWT = 401, member not found = 401, suspended = 403, cancelled = 403, active = returns member, non-admin = 403, admin = returns member (C1, V4.1.1) |
| `tests/server/api/auth-login.test.js` | 4 | Existing and non-existing emails return identical response shape and message, missing email = 400 (H8, V2.1.7) |
| `tests/server/api/members-profile-patch.test.js` | 7 | `helcimCustomerId`, `role`, `status`, `email`, `_id` blocked from `$set`; allowed fields (`pronouns`, `bio`, `studio`, etc.) and nested objects (`offering`, `lookingFor`) pass through (H6, V5.1.3) |
### Manual Test Cases
These items require browser or network-level verification and are not covered by automated tests:
**Item 1 (Cookie flags):** Open DevTools > Application > Cookies. Verify `auth-token` has `HttpOnly: true`. Run `document.cookie` in console -- `auth-token` must NOT appear. Auth flow must still work.
**Item 4 (Payment verification):** Fake `cardToken` = failure. Real HelcimPay.js flow in test mode = success.
**Item 5 (JWT fallback):** Unset `JWT_SECRET`, start server = crash with clear error. Set it = normal startup.
**Item 7 (Upload auth):** POST `/api/upload/image` with no cookie = 401. With valid auth = success.
**Item 8 (Payment auth):** Each endpoint with no cookie = 401. Full join flow still completes.
### Integration verification (after each phase)
- `npm run test:run` -- all 79 tests pass
- `npm run build` succeeds
- Full join flow: free tier + paid tier
- Full login flow: magic link request, click, redirect to `/members`
- Profile editing: avatar upload, bio update, privacy settings
- Admin pages: access control verified
- `curl` against hardened endpoints: unauthenticated = rejected