7.5 KiB
7.5 KiB
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 (labelTODO) ordocs/process/todo/entries linked toSEC-*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, plustenant.adminon sensitive routes). - Captured environment/header defaults:
.env.exampleships withAPP_DEBUG=true(must be false in prod) and HTTP APP_URL; session defaultssame_site=lax,SESSION_ENCRYPT=false, secure cookie flag unset; CORS uses Laravel defaults (all origins/methods, credentials=false) onapi/*; 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, defaultsadmin@example.com/ChangeMe123!), tenant admin demo (tenant-demo@fotospiel.app/Demo1234!), guest join token from demo event (W2E3sbt7yclzpkAwNSARHYTVN1sPLBad8hfUjLVHmjkUviPd), with seed commands (migrate --seedor targeted seeders). - Noted env assumptions for dynamic testing: set HTTPS
APP_URL, secure/samesite cookies, adjustSANCTUM_STATEFUL_DOMAINSfor SPA/PWA origins, ensure storage disks andstorage:linkready, 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 broadhttps:/data:allowances; plan inSEC-FE-01covers 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 fromCORS_ALLOWED_ORIGINS, app URL origin, dev hosts) and.env.exampledefaults. Prod should set exact domains; credentials stay off unless explicitly enabled.
Guest PWA upload/share
- Upload validation allows
imagetypes including SVG; files are stored and URLs returned viaStorage::urlwithout 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:
GuestPhotoUploadedlistener only sends notifications;securityconfig not wired. → Recommend enqueueing AV/EXIF jobs on upload before markingapproved/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_ORIGINSetc.), defaults to localhost; prod should set exact domains to match Traefik/nginx. - Security headers middleware added (
ResponseSecurityHeaders) for web/api: sets Referrer-Policystrict-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
mimesallowlist, sets uploads tostatus=pending(no file URLs in response), and dispatchesProcessPhotoSecurityScanon upload. Scan job now auto-approves on clean or skipped scans, rejects on infected. Gallery endpoints already filterstatus=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::urlwhere 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 addedframe-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 nonce’d 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 ~30–60 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_ORIGINSin prod/stage to match Traefik hosts when ready. - Logging/PII: current logging avoids raw tokens/paths; keep this guard in future changes.