jennie/ghostguild-org
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add Zod validation to all API endpoints and remove debug test route

Browse source 
Adds schema-based input validation across helcim, events, members,
series, admin, and updates API endpoints. Removes the peer-support
debug test endpoint. Adds validation test coverage.
This commit is contained in:
Jennie Robinson Faber 2026-03-01 17:04:26 +00:00
parent e4813075b7
commit 025c1a180f
38 changed files with 1132 additions and 309 deletions
Download patch file Download diff file Expand all files Collapse all files

101
docs/SECURITY_EVALUATION.md
View file

@ -1,6 +1,6 @@
# Ghost Guild Security Evaluation
**Date:** 2026-02-28 (updated 2026-03-01)
**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)
@ -132,14 +132,12 @@ ASVS Level 1 targets "all software" and is achievable for a small team.
|---|---|---|
| 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)
## Remediation Summary (Phases 0-2)
All work lives on branch `security/asvs-remediation`.
@ -175,6 +173,22 @@ All work lives on branch `security/asvs-remediation`.
### 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
@ -203,25 +217,48 @@ All work lives on branch `security/asvs-remediation`.
| 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
### 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 | 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 |
| 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: Before building planned features
### 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 |
|---|-----|--------|
| 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 |
| 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 |
---
@ -244,7 +281,7 @@ npm run test:run # Single run (CI)
- `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)
### Test Coverage (213 tests across 12 files)
| File | Tests | Security Controls Verified |
|------|-------|---------------------------|
@ -254,8 +291,12 @@ npm run test:run # Single run (CI)
| `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/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
@ -271,12 +312,38 @@ These items require browser or network-level verification and are not covered by
**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 79 tests pass
- `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.