Grow/SECURITY_AUDIT.md

Security Audit — Grow App

Date: 2026-06-15
Auditor: Claude Sonnet 4.6 (automated)
Scope: All API routes in src/app/api/

---

Summary

Severity	Found	Fixed
CRITICAL	9	9
HIGH	11	10
MEDIUM	7	5
LOW	1	0

Updated after second-pass audit (pass 2 of 2).

---

CRITICAL — Fixed

1. Missing family ownership check on baby data routes

Affected: events, growth, doctor-notes, journal, milestones, vaccinations, milk, search, export, reminders, teeth — all GET and POST handlers accepting babyId

Problem: Routes accepted any babyId from the caller and queried/wrote data without verifying the baby belongs to the authenticated user's family. An attacker with a valid session could enumerate any other family's data by guessing baby IDs (cuid values are not secret).

Fix: Added prisma.baby.findFirst({ where: { id: babyId, familyId } }) check before every Prisma query. Returns 404 if baby not owned by session family.

---

2. Missing ownership check on [id] mutation routes

Affected: events/[id], growth/[id], milestones/[id], vaccinations/[id], milk/[id], reminders/[id] — all PATCH and DELETE handlers

Problem: Routes updated or deleted records by bare ID with no ownership verification. Any authenticated user could destroy another family's data by providing the record ID.

Fix: Added findFirst({ where: { id, baby: { familyId } } }) before each PATCH/DELETE. Returns 404 if not owned.

---

3. Missing family check on journal [id] routes

Affected: journal/[id] PATCH and DELETE

Problem: Existing check verified note.userId === session.user.id (author check) but a PARENT from another family could modify any journal note they know the ID of (PARENT role bypasses the author check).

Fix: Added baby family check before the role/ownership check.

---

4. Missing family check on doctor-notes [id] routes

Affected: doctor-notes/[id] PATCH and DELETE

Problem: Same pattern — the note's baby family was never verified. A doctor from family A could modify notes for family B if they know the note ID.

Fix: Added baby family check before the userId ownership check.

---

5. baby/[id] PATCH — any authenticated user could modify any baby

Affected: baby/[id]/route.ts PATCH

Problem: No family or role check. Any authenticated user with a baby ID could rename or change birth dates for babies in other families.

Fix: Added familyId check + PARENT role requirement.

---

6. Admin config GET exposes sensitive secrets

Affected: admin/config/route.ts GET

Problem: The GET endpoint returned plaintext values for mailgun_api_key, smtp_pass, cron_secret, and pushover_app_token to any superadmin session. These secrets are write-only configuration.

Fix: Sensitive keys masked as •••••• in GET response. POST (update) still accepts and stores new values normally.

---

HIGH — Fixed

7. Photo files served without authentication

Affected: photos/[filename]/route.ts GET

Problem: Photo files were publicly accessible to anyone who knew (or guessed) the filename. Filenames include timestamps and short random suffixes — not truly unguessable.

Fix: Added auth() check; returns 404 (not 401) to avoid confirming file existence.

---

8. Missing PARENT role check on invite send-email

Affected: invite/send-email/route.ts POST

Problem: Any authenticated user (including READONLY viewers) could send family invitations, bypassing the intended restriction to PARENT role.

Fix: Added if (user.role !== "PARENT") return 403 after fetching the user record.

---

9. teeth/[code] DELETE — no family ownership check

Affected: teeth/[code]/route.ts DELETE

Problem: Took babyId from query param but didn't verify baby belongs to user's family.

Fix: Added baby family ownership check.

---

HIGH — Not fixed (by design / low exploitability)

10. Token enumeration via invite token endpoints

Affected: invite/token-info, invite/redeem

Observation: Different HTTP status codes for invalid (404) vs expired (410) tokens allow an attacker to determine whether a token ever existed. However: tokens are cuid() values (128 bits of entropy), making enumeration computationally infeasible. No action taken.

---

11. Admin config stores cron_secret plaintext in DB

Observation: Cron endpoints use a Bearer token stored in SystemConfig table (plaintext). Acceptable risk given superadmin-only DB access. Consider hashing in a future iteration.

---

MEDIUM — Fixed

12. reminders/[id] PATCH and DELETE — no ownership check

Fixed in batch with other [id] routes above.

---

MEDIUM — Not fixed (accepted risk / architectural)

13. Webhook secret returned in GET/LIST response

Observation: The POST (create) response returns the secret once — consistent with API key best practice. The GET list already excludes secret via Prisma select. Confirmed no leak.

14. v1 API events — type param not validated against allowlist

Observation: Prisma parameterizes the type value, so no SQL injection risk. However, clients could store arbitrary type strings. Low priority.

15. familyId in JWT not re-validated on each request

Observation: familyId is read from the JWT session without a DB round-trip. If a user is removed from a family in the DB, their token remains valid until expiration. Next-auth's default session expiry (30 days) makes this window wide. Mitigation: reduce maxAge in next-auth config, or add a session validation hook.

---

LOW

16. Superadmin check falls through if SUPERADMIN_EMAIL is unset

Affected: admin/config, admin/audit, admin/stats, admin/users, etc.

If SUPERADMIN_EMAIL env var is not set, the condition SUPERADMIN_EMAIL && session.user.email !== SUPERADMIN_EMAIL short-circuits to false, making the second condition (!user?.isSuperAdmin) the only gate. This is technically correct but relies on isSuperAdmin being set properly in the DB. Acceptable given the admin routes are superadmin-only by convention.

---

---

HIGH — Fixed (pass 2)

17. medication-profiles/[id] — no family ownership check

Affected: PATCH and DELETE

Problem: Any authenticated user could modify or delete any medication profile by ID.

Fix: Added prisma.medicationProfile.findFirst({ where: { id, familyId } }) check before PATCH and DELETE.

---

18. event-templates/[id] — no family ownership check

Affected: PATCH and DELETE

Problem: Any authenticated user could modify or delete any event template by ID.

Fix: Added prisma.eventTemplate.findFirst({ where: { id, familyId } }) check before PATCH and DELETE.

---

19. notify/push — no family ownership check on babyId

Problem: Baby was fetched by ID but its familyId was never compared against the session user's family. Any authenticated user could trigger push notifications using another family's baby ID (leaking feeding status via the notification message).

Fix: Added if (baby.familyId !== sessionFamilyId) return 404.

---

20. events/route.ts GET — event type not validated against allowlist

Problem: The type query param was passed directly into the Prisma where clause without validation. While Prisma parameterizes values (no SQL injection), arbitrary strings could be stored and returned.

Fix: Imported ALL_EVENT_TYPES from event-config.ts and added if (type && !ALL_EVENT_TYPES.includes(type)) return 400.

---

21. events/route.ts GET — limit/offset not sanitized

Problem: parseInt() returns NaN on non-numeric input; NaN passed to Prisma take/skip causes an error or is treated as 0, potentially returning all rows.

Fix: Added clamping: limit = Math.max(1, Math.min(parseInt(limit) || 50, 500)), offset = Math.max(0, parseInt(offset) || 0).

---

MEDIUM — Fixed (pass 2)

22. v1/summary — arithmetic operator precedence bug

Problem: counts.BREASTFEED ?? 0 + (counts.BOTTLE ?? 0) evaluated as counts.BREASTFEED ?? (0 + (counts.BOTTLE ?? 0)) due to ?? precedence, causing incorrect feed count.

Fix: Added explicit parentheses: (counts.BREASTFEED ?? 0) + (counts.BOTTLE ?? 0).

---

23. v1/events GET — offset not sanitized (pass 2)

Fix: Math.max(0, parseInt(offset) || 0). Also fixed limit: Math.min of NaN returns NaN; added || 50 fallback.

---

24. v1/growth POST — date not validated before new Date()

Fix: Added if (isNaN(new Date(date).getTime())) return 400.

---

MEDIUM — Not applicable (already handled)

Upload route — extension from MIME type, not filename

The audit flagged upload/route.ts for unsafe extension extraction. The route derives the extension from file.type (e.g., "image/jpeg".split("/")[1]) — not from the original filename — and ALLOWED_TYPES.includes(file.type) is checked before that line. No fix needed.

---

Routes verified as already secure

• webhooks/route.ts — familyId from session, no cross-family access possible
• webhooks/[id]/route.ts — findFirst({ where: { id, familyId } }) already present
• invite/pending/[id]/route.ts — familyId from DB lookup before delete
• family/reset-invite/route.ts — PARENT role check present
• medication-profiles/route.ts — familyId from DB lookup, fully scoped
• api-keys/route.ts — familyId scoped
• All admin/* routes — superadmin check before any action
• All cron/* routes — CRON_SECRET bearer token required

---

Recommendations (not implemented)

1. Rate limiting on auth endpoints (/api/auth/*, /api/register, /api/invite/*) — use @upstash/ratelimit or a middleware layer.
2. Session TTL reduction — default 30d is long; consider 7d with sliding expiry.
3. Audit log on all DELETE operations — currently only events have logAudit.
4. CORS policy — verify NEXTAUTH_URL is set to prevent open redirects on OAuth callbacks.