diff --git a/docs/SECURITY_EVALUATION.md b/docs/SECURITY_EVALUATION.md new file mode 100644 index 0000000..b337246 --- /dev/null +++ b/docs/SECURITY_EVALUATION.md @@ -0,0 +1,282 @@ +# 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, ` world') + expect(result).not.toContain('') + expect(result).toContain('Hello') + expect(result).toContain('world') + }) + + it('strips onerror attributes', () => { + const result = render('') + expect(result).not.toContain('onerror') + }) + + it('strips onclick attributes', () => { + const result = render('click') + expect(result).not.toContain('onclick') + }) + + it('strips iframe tags', () => { + const result = render('') + expect(result).not.toContain(' { + const result = render('') + expect(result).not.toContain(' { + const result = render('') + expect(result).not.toContain(' { + const result = render('[click me](javascript:alert(1))') + expect(result).not.toContain('javascript:') + }) + + it('strips img tags (not in allowed list)', () => { + const result = render('![alt](https://example.com/img.png)') + expect(result).not.toContain(' { + it('renders bold and italic', () => { + const result = render('**bold** and *italic*') + expect(result).toContain('bold') + expect(result).toContain('italic') + }) + + it('renders links with href', () => { + const result = render('[Ghost Guild](https://ghostguild.org)') + expect(result).toContain(' { + for (let i = 1; i <= 6; i++) { + const hashes = '#'.repeat(i) + const result = render(`${hashes} Heading ${i}`) + expect(result).toContain(``) + } + }) + + it('preserves code blocks', () => { + const result = render('`inline code` and\n\n```\nblock code\n```') + expect(result).toContain('') + expect(result).toContain('
')
+    })
+
+    it('preserves blockquotes', () => {
+      const result = render('> This is a quote')
+      expect(result).toContain('
')
+    })
+
+    it('preserves lists', () => {
+      const result = render('- item 1\n- item 2')
+      expect(result).toContain('