ghostguild-org/docs/SECURITY_EVALUATION.md

# Ghost Guild Security Evaluation

**Date:** 2026-02-28 (Phases 0-3 complete as of 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 |
| 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-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`.

### Input validation (`server/utils/schemas.js` + `server/utils/validateBody.js`)
- Centralized Zod schemas for all API endpoints. `validateBody(event, schema)` replaces `readBody` and returns only validated fields (stripping unknown properties).
- `memberCreateSchema`: Explicit allowlist (email, name, circle, contributionTier). Prevents mass assignment of `role`, `status`, `helcimCustomerId`, `_id`.
- `emailSchema`: Trims, lowercases, and validates email format.
- `memberProfileUpdateSchema`: Type/length validation on all profile fields and privacy enum values.
- `updateCreateSchema`: Content length limit (1–50000 chars), image URL array (max 20).
- `adminEventCreateSchema`: Required title, description, dates; optional capacity/location/pricing/tickets.
- `paymentVerifySchema`: Validates cardToken and customerId presence.

### Additional hardening
- Logout cookie flags now match login flags (`httpOnly: true`, `secure` conditional on NODE_ENV).
- Removed 3 unauthenticated test/debug endpoints (`test-connection`, `test-subscription`, `test-bot`).
- Removed sensitive `console.log` statements from Helcim and member creation endpoints.
- Removed unused `bcryptjs` dependency.
- Added 10MB file size limit on image uploads.

---

## 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) -- COMPLETE

| # | Finding | Fix | ASVS | Status |
|---|---------|-----|------|--------|
| 15 | H7 | Implement Zod validation across all API endpoints | V5.1.3 | Done |
| 16 | M5 | Escape regex in directory search | V5.3.4 | Done |
| 17 | M4 | Remove sensitive console.log statements | V7.1.1 | Done |
| 18 | M3 | Make devtools conditional on NODE_ENV | V7.4.1 | Done |
| 19 | M1 | Shorter session tokens (7d) with refresh endpoint | V2.5.2 | Done |
| 20 | -- | Create shared `requireAuth()`/`requireAdmin()` utilities | V4.1.1 | Done |
| 21 | -- | Fix mass assignment in member creation (`new Member(body)`) | V5.1.3 | Done |
| 22 | -- | Fix logout cookie flags to match login (httpOnly, secure) | V3.2.1 | Done |
| 23 | -- | Remove unauthenticated test/debug endpoints | V4.1.1 | Done |
| 24 | -- | Remove dead `bcryptjs` dependency | -- | Done |
| 25 | -- | Add 10MB file size limit on image uploads | V13.1.1 | Done |

### Phase 3: Remaining hardening -- COMPLETE

| # | Severity | Fix | ASVS | Status |
|---|----------|-----|------|--------|
| 26 | HIGH | `create-plan.post.js` has no auth guard -- anyone can create Helcim payment plans | V4.1.1 | Done |
| 27 | HIGH | `plans.get.js` and `subscriptions.get.js` have no auth -- expose Helcim plan/subscription data | V4.1.1 | Done |
| 28 | MEDIUM | `create.post.js` returns full Mongoose member document in response (leaks `role`, `status`, `helcimCustomerId`, internal fields) | V5.1.2 | Done |
| 29 | MEDIUM | Helcim error text forwarded to client in `statusMessage` across 7 endpoints (`create-plan`, `customer-code`, `initialize-payment`, `subscription`, `update-billing`, `customer`, `get-or-create-customer`) | V7.4.1 | Done |
| 30 | MEDIUM | `tickets/purchase.post.js` and 24 other endpoints still use raw `readBody` without Zod validation | V5.1.3 | Done |
| 31 | LOW | `server/api/test/peer-support-debug.get.js` is a debug endpoint in a `test/` directory -- has auth but should be removed before production | V14.2.2 | Done |

**Phase 3 remediation details:**
- Items 26-27: Added `await requireAdmin(event)` to `create-plan.post.js`, `plans.get.js`, and `subscriptions.get.js`.
- Item 28: Member creation response now returns an explicit projection (`id`, `email`, `name`, `circle`, `contributionTier`, `status`). Also fixed `subscription.post.js` (5 return paths) and `update-contribution.post.js` (4 return paths) to not expose full member documents. Fixed `get-or-create-customer.post.js` error text forwarding.
- Item 29: Replaced all `` `Failed to X: ${errorText}` `` patterns with generic client messages. Also fixed outer catch blocks that forwarded `error.message` -- these now re-throw known `createError` instances and use generic messages for unknown errors.
- Item 30: Added 25 new Zod schemas to `server/utils/schemas.js` and migrated all 25 endpoints from `readBody` to `validateBody`. Total schema count: 32 (7 from Phase 2 + 25 from Phase 3). All POST/PUT/PATCH/DELETE endpoints with request bodies now use Zod validation.
- Item 31: Deleted `server/api/test/peer-support-debug.get.js` and the `server/api/test/` directory.

### Phase 4: Before building planned features

| # | Fix | Status |
|---|-----|--------|
| 32 | Build sanitization utility (DOMPurify wrapper) for all user-generated HTML | Open |
| 33 | Design admin role model with granular permissions | Open |
| 34 | Implement file validation pipeline (type, size, virus scanning) | Open |
| 35 | 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 (213 tests across 12 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 via Zod (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) |
| `tests/server/api/validation.test.js` | 38 | Mass assignment: `role`, `status`, `helcimCustomerId`, `_id` stripped from member create; email format validation on login/register/create; content length limits; image array limits; payment token validation; enum validation for circles, tiers, privacy; `validateBody` integration (H7, V5.1.3) |
| `tests/server/api/helcim-auth.test.js` | 6 | `create-plan.post.js`, `plans.get.js`, `subscriptions.get.js` all call `requireAdmin` before business logic (Items 26-27, V4.1.1) |
| `tests/server/api/members-create-response.test.js` | 9 | Response projection: no raw `member` return, explicit `_id`/`email`/`name`/`circle`/`status` fields, no `helcimCustomerId` or `role` in response, no `error.message` forwarding (Item 28, V5.1.2) |
| `tests/server/api/validation-phase3.test.js` | 81 | 25 new Zod schemas: invalid input rejected, valid input passes, unknown fields stripped, mass assignment prevented; error text forwarding regression checks on 8 Helcim endpoints; validateBody migration verified across all 25 migrated endpoints (Items 29-30, V5.1.3, V7.4.1) |

### 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.

**Item 21 (Mass assignment):** `curl -X POST /api/members/create -d '{"email":"test@test.com","name":"Test","circle":"community","contributionTier":"0","role":"admin"}' -H 'Content-Type: application/json'` -- response member must have `role: "member"`, not `"admin"`.

**Item 22 (Logout cookie):** Log in, verify `auth-token` cookie exists. Log out, verify `auth-token` cookie is cleared. `document.cookie` must not contain `auth-token` at any point.

**Item 23 (Deleted endpoints):** `curl /api/helcim/test-connection`, `/api/helcim/test-subscription`, `/api/slack/test-bot` must all return 404.

**Item 25 (Upload size):** Upload a file >10MB to `/api/upload/image` -- must return 400 "File too large".

### Integration verification (after each phase)

- `npm run test:run` -- all 213 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

---

## Notes for Future Rounds

### Zod v4 behavior to be aware of

- **Transform ordering matters.** `.trim().toLowerCase()` must come BEFORE `.email()`. Zod v4 runs transforms in chain order, so `.email().trim()` validates the untrimmed string first and rejects `"  user@example.com  "`. This is opposite to some other validation libraries.
- **`.optional()` does NOT accept `null`.** Only `undefined`. If the frontend sends `null` to clear a field, Zod rejects it. Use `.nullable()` or `.nullish()` if null-clearing is needed. Audit frontend form behavior before migrating remaining endpoints.
- **Unknown keys are stripped, not rejected.** `z.object()` silently drops fields not in the schema. This is the desired behavior for mass assignment prevention, but means typo'd field names from the frontend will be silently ignored rather than producing an error. Use `.strict()` on schemas if you want to reject unexpected fields.

### Patterns that recurred during Phases 2-3

- **Auth guard gaps.** (Fixed in Phase 3, items 26-27.) Every new endpoint needs `requireAuth` or `requireAdmin`. Recommendation: add a grep check to CI that flags any `server/api/` handler not containing `requireAuth`, `requireAdmin`, or an explicit `// @public` comment.
- **Error text forwarding.** (Fixed in Phase 3, item 29.) The pattern `` statusMessage: `Failed to X: ${errorText}` `` leaks upstream API error details to the client. Standard pattern is now: generic client message + `console.error` for server-side logging.
- **Response over-exposure.** (Fixed in Phase 3, item 28.) `return { success: true, member }` sends the full Mongoose document. Standard pattern is now: explicit projection to allowlisted fields.
- **`readBody` audit.** (Fixed in Phase 3, item 30.) All POST/PUT/PATCH/DELETE endpoints now use `validateBody` with Zod schemas. 32 total schemas across 32 endpoints.
- **Debug endpoints.** (Fixed in Phase 3, item 31.) `server/api/test/` directory deleted. Consider a build-time check or `.gitignore` pattern to prevent future `server/api/test/` additions.