Files
fotospiel-app/docs/process/changes/2025-12-08-security-review-kickoff.md
2025-12-09 20:36:35 +01:00

51 lines
7.5 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Security Review Kickoff — 2025-12-08
## Context
- Started structured security review spanning marketing/public API, Guest PWA, Event Admin, payments/webhooks, and media pipeline/storage.
- Needed a tracking checklist plus logging conventions for findings and follow-ups.
## Actions
- Added review plan with scope, workstreams, and checklists: `docs/process/todo/security-review-dec-2025.md`.
- Defined evidence logging: future session notes in `docs/process/changes/`, remediation tasks as Issues (label `TODO`) or `docs/process/todo/` entries linked to `SEC-*` tickets where applicable.
- Mapped roles/trust boundaries and inventoried marketing/public API route surfaces (locale-prefixed marketing group with throttled contact forms and signed gift voucher print; guest PWA view routes; Event Admin SPA catch-all; checkout/paddle webhook exposure; API v1 marketing + public event endpoints with throttles; tenant API protected by `auth:sanctum`, `tenant.collaborator`, `tenant.isolation`, `throttle:tenant-api`, plus `tenant.admin` on sensitive routes).
- Captured environment/header defaults: `.env.example` ships with `APP_DEBUG=true` (must be false in prod) and HTTP APP_URL; session defaults `same_site=lax`, `SESSION_ENCRYPT=false`, secure cookie flag unset; CORS uses Laravel defaults (all origins/methods, credentials=false) on `api/*`; CSP middleware added to web group sets nonce-based script-src but leaves style-src with `'unsafe-inline'` and broad https/data allowances; skips CSP when debug/local.
- Added data class/retention sketch: media storage (variants via signed URLs; tenant-governed retention), join tokens/access logs and planned hashing, auth/session tokens (Sanctum/PATs, OAuth), PII (contact form, member lists, billing contact), billing identifiers (Paddle/Stripe/PayPal), logs/analytics (Matomo consent plan, request timing middleware).
- Captured seeded test identities: Super admin (`ADMIN_EMAIL`/`ADMIN_PASSWORD`, defaults `admin@example.com` / `ChangeMe123!`), tenant admin demo (`tenant-demo@fotospiel.app` / `Demo1234!`), guest join token from demo event (`W2E3sbt7yclzpkAwNSARHYTVN1sPLBad8hfUjLVHmjkUviPd`), with seed commands (`migrate --seed` or targeted seeders).
- Noted env assumptions for dynamic testing: set HTTPS `APP_URL`, secure/samesite cookies, adjust `SANCTUM_STATEFUL_DOMAINS` for SPA/PWA origins, ensure storage disks and `storage:link` ready, supply webhook secrets/URLs, and run queues for media/security jobs.
- Mapped role→data/retention and drafted dynamic testing harness scope (marketing/API abuse checks, guest PWA upload/share/cache tests, event admin CRUD/IDOR checks, webhook replay for freshness/idempotency, media pipeline AV/EXIF tests).
- Converted the testing outline into actionable checklists per surface (marketing/API, guest PWA, event admin, webhooks/billing, media pipeline, cross-cutting headers/CSRF/rate limits).
- Added guest upload gating plan (scan-before-publish, auto-approve when clean; optional manual moderation flag; signed URLs/private storage, API changes).
## Next Steps
- [ ] Start executing checklists (begin with marketing/API headers/CSP/CORS + guest PWA upload/share flows) and log findings with PoCs and severities.
## Findings — Marketing/API headers & Guest PWA upload/share (2025-12-08)
**Marketing/API headers & CORS**
- Missing security headers: no HSTS, Referrer-Policy, Permissions-Policy, or frame-ancestors/X-Frame-Options on responses; CSP middleware only for web group and skipped in debug/local (prod only). → Recommend adding a response headers middleware to set HSTS (HTTPS-only), Referrer-Policy (`strict-origin-when-cross-origin`), Permissions-Policy (disable camera/mic/geolocation unless required), and frame-ancestors (`'none'` or app domain).
- CSP style-src still uses `'unsafe-inline'` with broad `https:`/`data:` allowances; plan in `SEC-FE-01` covers nonce/hash rollout. Needs prioritization to remove inline allowance and tighten connect/img/font lists.
- CORS was default-open; added env-driven allowlist in `config/cors.php` (origin list from `CORS_ALLOWED_ORIGINS`, app URL origin, dev hosts) and `.env.example` defaults. Prod should set exact domains; credentials stay off unless explicitly enabled.
**Guest PWA upload/share**
- Upload validation allows `image` types including SVG; files are stored and URLs returned via `Storage::url` without sanitisation. SVGs can carry script and would be served from first-party origin, enabling stored XSS via shared links/gallery. → Recommend rejecting SVG (or sanitising server-side) for guest uploads.
- No antivirus/EXIF scrubbing triggered in upload flow: `GuestPhotoUploaded` listener only sends notifications; `security` config not wired. → Recommend enqueueing AV/EXIF jobs on upload before marking `approved`/serving assets.
- Uploaded assets are immediately marked `status='approved'` and URLs returned (likely public if disk is public). Confirm disk visibility; if public, anyone with URL can fetch regardless of token. → Recommend using signed URLs/private visibility and deferring publication until approval/security scan succeeds.
**Mitigations implemented (2025-12-08)**
- CORS tightened: new env-driven allowlist in `config/cors.php` (`CORS_ALLOWED_ORIGINS` etc.), defaults to localhost; prod should set exact domains to match Traefik/nginx.
- Security headers middleware added (`ResponseSecurityHeaders`) for web/api: sets Referrer-Policy `strict-origin-when-cross-origin`, X-Content-Type-Options nosniff, X-Frame-Options SAMEORIGIN, Permissions-Policy (camera/mic/geo disabled), and HSTS on secure non-local requests.
- Guest upload hardened: rejects SVG via `mimes` allowlist, sets uploads to `status=pending` (no file URLs in response), and dispatches `ProcessPhotoSecurityScan` on upload. Scan job now auto-approves on clean or skipped scans, rejects on infected. Gallery endpoints already filter `status=approved`, so pending items are hidden until scan finishes.
- Gallery/API assets moving to signed access: gallery listings and stats now use temporary signed routes for thumbnails/full URLs (token + photo id) instead of raw `Storage::url` where possible; queries filter to approved status. Fallbacks remain for legacy paths.
- CSP tightened: added style nonce, allowed https style sources for Stripe/Paddle, removed `style-src 'unsafe-inline'` in non-dev (dev keeps inline for Vite), and added `frame-ancestors 'self'`. Script nonce already in place.
- Branding assets signed: added signed branding asset route with path allowlist; branding logos use signed URLs; blog banners now emit signed URLs instead of raw `Storage::url`. Tenant photo resource now emits signed URLs for full/thumbnail variants.
- Paddle webhook throttled: added `throttle:paddle-webhook` (30/min per IP).
- Inline scripts/styles in guest/admin blades now carry nonces; inline styles consolidated into nonced blocks.
- Backfill thumbnails stores relative paths (no public URLs).
- Data export downloads remain auth-gated; added existence check and private/no-store headers on download.
**Remaining (low priority)**
- Signed URL TTL/scoping: can shorten TTLs (gallery/branding) and bind signatures to token/event for stricter replay protection; current defaults ~3060 mins are acceptable but could be reduced.
- Guest asset throttles: consider throttles on gallery asset/download/share routes for abuse mitigation; not critical if monitoring is in place.
- CORS prod allowlist: env-driven config exists; set `CORS_ALLOWED_ORIGINS` in prod/stage to match Traefik hosts when ready.
- Logging/PII: current logging avoids raw tokens/paths; keep this guard in future changes.