crewli/dev-docs/SECURITY_AUDIT.md

# Crewli Security Audit — 2026-04-14

## Summary

- **Total findings: 57**
- **Critical: 10 | High: 19 | Medium: 18 | Low: 10**

Audit scope: all files under `api/` and `apps/` (app, portal).

---

## Findings

---

### A01 — Broken Access Control

#### [CRITICAL] A01-1: `PortalTokenMiddleware` is a complete no-op

- **File:** `api/app/Http/Middleware/PortalTokenMiddleware.php:17-19`
- **Description:** The `portal.token` middleware calls `$next($request)` unconditionally — it performs zero token validation.
- **Risk:** Any route protected by `portal.token` is completely unprotected. Future routes added under this middleware will silently allow unauthenticated access.
- **Fix:** Implement the middleware to extract and validate a portal token from the request, establish the portal context (actor identity + permitted event), and return 401 if invalid.

#### [CRITICAL] A01-2: `PortalShiftController::claim()` does not verify the shift belongs to the event

- **File:** `api/app/Http/Controllers/Api/V1/Portal/PortalShiftController.php:173-202`
- **Description:** Route `POST /portal/events/{event}/shifts/{shift}/claim` resolves `$person` by `user_id + event_id` but does NOT verify that `$shift` belongs to `$event`. A volunteer can supply any shift ID from a different event or organisation.
- **Risk:** Cross-tenant shift claiming — volunteers can claim shifts in events/organisations they have no association with.
- **Fix:** Verify `$shift->festivalSection->event_id` is within the resolved event IDs before proceeding.

#### [CRITICAL] A01-3: `ShiftAssignmentPolicy` does not verify the assignment belongs to the event

- **File:** `api/app/Policies/ShiftAssignmentPolicy.php:19-43`
- **Description:** The `approve`, `reject`, and `cancel` policy methods check whether the user can manage `$event`, but never verify `$assignment` belongs to that event. A malicious org_admin for event A can approve/reject/cancel assignments from event B.
- **Risk:** Cross-event data manipulation across organisations.
- **Fix:** Add `if ($assignment->shift->festivalSection->event_id !== $event->id) return false;` in each method.

#### [HIGH] A01-4: `OrganisationScope` is defined but never registered on any model

- **File:** `api/app/Models/Scopes/OrganisationScope.php`
- **Description:** The scope class exists but no model registers it via `booted()` / `addGlobalScope()`. The CLAUDE.md mandates using it on all event-related models.
- **Risk:** No automatic multi-tenancy safety net. Isolation relies entirely on controller-level routing, which is fragile.
- **Fix:** Register the scope on `Event`, `Person`, `Shift`, `FestivalSection`, `TimeSlot`, `Location`, `CrowdList`, `CrowdType`, `Company`, `PersonTag`, `RegistrationFormField`, `RegistrationFieldTemplate`.

#### [HIGH] A01-5: `ShiftController::assign()` and `claim()` use unscoped `Person::findOrFail()`

- **File:** `api/app/Http/Controllers/Api/V1/ShiftController.php:74,84`
- **File:** `api/app/Http/Requests/Api/V1/AssignShiftRequest.php:20`
- **Description:** `Person::findOrFail($request->validated('person_id'))` fetches any person by ID regardless of event or organisation. The `exists:persons,id` rule has no scoping.
- **Risk:** Cross-organisation person assignment — an org admin can assign shifts to persons from another tenant.
- **Fix:** Scope to event: `Person::where('id', $personId)->where('event_id', $event->id)->firstOrFail()`.

#### [HIGH] A01-6: `CrowdListController::addPerson()` uses unscoped `Person::findOrFail()`

- **File:** `api/app/Http/Controllers/Api/V1/CrowdListController.php:81`
- **File:** `api/app/Http/Requests/Api/V1/AddPersonToCrowdListRequest.php:20`
- **Description:** Same issue as A01-5. A person from another organisation can be added to a crowd list.
- **Risk:** Cross-tenant data leakage and reference pollution.
- **Fix:** Scope the `exists` rule: `Rule::exists('persons', 'id')->where('event_id', $request->route('event')->id)`.

#### [HIGH] A01-7: `BulkApproveShiftAssignmentRequest` uses unscoped `exists:shift_assignments,id`

- **File:** `api/app/Http/Requests/Api/V1/BulkApproveShiftAssignmentRequest.php:21`
- **File:** `api/app/Http/Controllers/Api/V1/ShiftAssignmentController.php:98-103`
- **Description:** `assignment_ids.*` validated with `exists:shift_assignments,id` without event or organisation scope. The controller loads assignments without filtering.
- **Risk:** Cross-tenant bulk approval of shift assignments.
- **Fix:** Add `.whereHas('shift.festivalSection', fn ($q) => $q->where('event_id', $event->id))` to the query.

#### [HIGH] A01-8: `BulkConfirmIdentityMatchesRequest` uses unscoped `exists:person_identity_matches,id`

- **File:** `api/app/Http/Requests/Api/V1/BulkConfirmIdentityMatchesRequest.php:21`
- **Description:** Match IDs are not scoped to the route's organisation. An org admin could include IDs from another organisation.
- **Risk:** Cross-tenant identity match manipulation.
- **Fix:** Scope the query to organisation events in the controller.

#### [HIGH] A01-9: `ReorderRegistrationFormFieldsRequest` uses unscoped `exists:registration_form_fields,id`

- **File:** `api/app/Http/Requests/Api/V1/ReorderRegistrationFormFieldsRequest.php:21`
- **Description:** Field IDs are not scoped to the event. Fields from other events could be reordered.
- **Risk:** Cross-event field manipulation.
- **Fix:** Add `Rule::exists('registration_form_fields', 'id')->where('event_id', $this->route('event')->id)`.

#### [HIGH] A01-10: `PortalMeController::index()` accepts any `event_id` without access verification

- **File:** `api/app/Http/Controllers/Api/V1/PortalMeController.php:23-83`
- **Description:** Takes `event_id` as a query parameter and calls `Event::findOrFail()` which returns any event from any organisation.
- **Risk:** A user registered for events at multiple organisations can access their profile data cross-org by probing event IDs.
- **Fix:** Validate that the authenticated user has a Person record for the requested event.

#### [HIGH] A01-11: `InvitationController::revoke()` does not verify the invitation belongs to the organisation

- **File:** `api/app/Http/Controllers/Api/V1/InvitationController.php:76`
- **Description:** Route `DELETE organisations/{organisation}/invitations/{invitation}` resolves the invitation globally. Never checks `$invitation->organisation_id === $organisation->id`.
- **Risk:** An org admin of Org A can revoke invitations belonging to Org B.
- **Fix:** Add `if ($invitation->organisation_id !== $organisation->id) abort(404);`.

#### [HIGH] A01-12: `PersonIdentityMatchController` does not verify matches belong to the route organisation

- **File:** `api/app/Http/Controllers/Api/V1/PersonIdentityMatchController.php:51,66,85`
- **Description:** `confirm`, `dismiss`, and `bulkConfirm` resolve matches globally. The route's `{organisation}` parameter is unused in policy checks.
- **Risk:** Cross-org identity match confirmation/dismissal.
- **Fix:** Scope match resolution: `->whereHas('person', fn ($q) => $q->whereIn('event_id', $organisation->events()->select('id')))`.

#### [HIGH] A01-13: Event routes without organisation prefix bypass scope

- **File:** `api/routes/api.php:152-207`
- **Description:** Routes like `events/{event}/stats`, `events/{event}/locations`, etc. are not nested under `organisations/{organisation}`. The Event model has no OrganisationScope. Route model binding finds events globally.
- **Risk:** Authenticated users can access event data from other organisations. The `stats` endpoint specifically calls `Gate::authorize('view', $event)` with no organisation parameter, skipping the org ownership check in the policy.
- **Fix:** Move routes under `organisations/{organisation}/events/{event}/...` or register OrganisationScope on the Event model.

#### [MEDIUM] A01-14: Multiple form requests use unscoped `exists:crowd_types,id` and `exists:companies,id`

- **File:** `api/app/Http/Requests/Api/V1/StorePersonRequest.php:20,26`
- **File:** `api/app/Http/Requests/Api/V1/UpdatePersonRequest.php:20,26`
- **File:** `api/app/Http/Requests/Api/V1/StoreCrowdListRequest.php:22,25`
- **File:** `api/app/Http/Requests/Api/V1/UpdateCrowdListRequest.php:22,25`
- **Description:** These allow `crowd_type_id` and `company_id` values from any organisation.
- **Risk:** Cross-tenant reference pollution.
- **Fix:** Use `Rule::exists()->where('organisation_id', ...)`.

#### [MEDIUM] A01-15: `UpdateEventRequest` uses unscoped `exists:events,id` for `parent_event_id`

- **File:** `api/app/Http/Requests/Api/V1/UpdateEventRequest.php:26`
- **Description:** `parent_event_id` validated with `exists:events,id` without organisation scoping. The update controller does not re-verify.
- **Risk:** An event can be re-parented to an event from a different organisation.
- **Fix:** Scope: `Rule::exists('events', 'id')->where('organisation_id', $this->route('organisation')->id)`.

#### [MEDIUM] A01-16: `PortalShiftController::cancel()` does not verify `shiftAssignment` belongs to the event

- **File:** `api/app/Http/Controllers/Api/V1/Portal/PortalShiftController.php:204-237`
- **Description:** Checks `person_id` ownership but not that the assignment belongs to the URL's event context.
- **Risk:** Cross-event assignment cancellation for users registered in multiple events.
- **Fix:** Verify `$shiftAssignment->shift->timeSlot->event_id` is in the resolved event IDs.

#### [MEDIUM] A01-17: `MemberController::update()` and `destroy()` don't verify the user is a member

- **File:** `api/app/Http/Controllers/Api/V1/MemberController.php:27-82`
- **Description:** Neither verifies the target `$user` is actually a member of the organisation. The "last org_admin" safeguard can be bypassed.
- **Risk:** Misleading success responses; bypasses admin count safeguard.
- **Fix:** Add `if (!$organisation->users()->where('user_id', $user->id)->exists()) abort(404);`.

#### [MEDIUM] A01-18: `RegistrationFormFieldController::importFromEvent()` fragile org check

- **File:** `api/app/Http/Controllers/Api/V1/RegistrationFormFieldController.php:98-107`
- **File:** `api/app/Http/Requests/Api/V1/ImportFromEventRequest.php:20-40`
- **Description:** The `exists:events,id` rule accepts any event. Cross-org check is in `withValidator` callback, not in the rule itself.
- **Risk:** Fragile defence — removing the `withValidator` callback silently enables cross-org import.
- **Fix:** Use `Rule::exists('events', 'id')->where('organisation_id', ...)` directly in the rules array.

#### [LOW] A01-19: `OrganisationPolicy::viewAny()` returns `true` unconditionally

- **File:** `api/app/Policies/OrganisationPolicy.php:13-16`
- **Description:** Returns `true` for all authenticated users. Controller correctly scopes results, but the policy provides no guard.
- **Risk:** No current exposure; maintenance risk if controller query changes.
- **Fix:** Document the design decision; optionally remove the policy gate call.

#### [LOW] A01-20: `ShiftAssignmentController::assignablePersons()` uses raw `DB::table()` query

- **File:** `api/app/Http/Controllers/Api/V1/ShiftAssignmentController.php:156-170`
- **Description:** Raw query correctly scoped but bypasses Eloquent models and future global scopes.
- **Risk:** Maintenance risk.
- **Fix:** Replace with Eloquent query.

---

### A02 — Cryptographic Failures

#### [CRITICAL] A02-1: Portal and invitation tokens use ULID (predictable time-prefix)

- **File:** `api/database/migrations/2026_04_08_170000_create_artists_table.php:31`
- **File:** `api/app/Services/InvitationService.php:45`
- **Description:** Portal tokens are `char(26)` (ULID size). ULID has 48 predictable timestamp bits + only 80 random bits. Invitation tokens also use `Str::ulid()`.
- **Risk:** An attacker who knows the approximate creation time can reduce the search space significantly. Combined with no rate limiting (A07-5), token brute-force becomes practical.
- **Fix:** Use `bin2hex(random_bytes(32))` for all security tokens — provides 256 bits of cryptographic randomness.

#### [HIGH] A02-2: Sanctum tokens never expire

- **File:** `api/config/sanctum.php:52`
- **Description:** `'expiration' => null` — all issued tokens are permanent.
- **Risk:** A stolen token grants indefinite access with no automatic remediation.
- **Fix:** Set `'expiration' => 60 * 24 * 7` (7 days). Implement token refresh for long-lived sessions.

#### [HIGH] A02-3: Portal tokens stored plaintext with no expiry or revocation

- **File:** `api/app/Http/Controllers/Api/V1/PortalTokenController.php`
- **File:** `api/database/migrations/2026_04_08_170000_create_artists_table.php:31`
- **Description:** Portal tokens are compared with `where('portal_token', $token)` — plaintext storage. No `expires_at` column, no revocation mechanism.
- **Risk:** A database dump exposes all active tokens. A compromised token is valid indefinitely.
- **Fix:** Store `hash('sha256', $token)`, add `portal_token_expires_at` column, expire tokens after the event ends.

#### [MEDIUM] A02-4: Session `secure` cookie not enforced

- **File:** `api/config/session.php:172`
- **Description:** `'secure' => env('SESSION_SECURE_COOKIE')` resolves to `null`/`false` when env var is absent. Session cookie sent over plain HTTP.
- **Risk:** Session hijacking via HTTP interception.
- **Fix:** Add `SESSION_SECURE_COOKIE=true` to `.env.example`. Default: `env('SESSION_SECURE_COOKIE', app()->isProduction())`.

#### [LOW] A02-5: Session encryption disabled by default

- **File:** `api/.env:28`
- **Description:** `SESSION_ENCRYPT=false` — session data in database is unencrypted.
- **Risk:** Database access exposes session contents.
- **Fix:** Set `SESSION_ENCRYPT=true` in production.

#### [LOW] A02-6: No HTTPS enforcement middleware

- **File:** `api/config/app.php:55`
- **Description:** `APP_URL` defaults to `http://` and no `ForceHttps` middleware exists.
- **Risk:** API accessible over HTTP in misconfigured deployments.
- **Fix:** Add HTTPS enforcement middleware for production, or use `URL::forceScheme('https')` in `AppServiceProvider`.

---

### A03 — Injection

#### [MEDIUM] A03-1: `EventController::index()` uses unvalidated query parameters in `where()` clauses

- **File:** `api/app/Http/Controllers/Api/V1/EventController.php:31-33`
- **Description:** Unvalidated query param `type` used in `where()` — no Form Request for index action. While Eloquent uses PDO bindings (no SQL injection), this is an input validation gap.
- **Risk:** Unexpected behaviour with malformed input.
- **Fix:** Add a Form Request or validate `type` as `in:event,festival,series`.

#### [LOW] A03-2: `PersonController::reject()` — unvalidated `reason` passed to mail

- **File:** `api/app/Http/Controllers/Api/V1/PersonController.php:130`
- **Description:** `$request->input('reason')` used without length validation. The Blade template escapes it (`{{ }}`), so no XSS, but no length cap exists.
- **Risk:** Oversized email payload.
- **Fix:** Add a Form Request with `'reason' => ['nullable', 'string', 'max:1000']`.

#### Positive findings (A03)

- All `whereRaw()` / `selectRaw()` calls use parameterised bindings — no SQL injection vectors found.
- No `exec()`, `shell_exec()`, `system()`, `proc_open()`, `passthru()`, or backtick operators found.
- No `{!! !!}` (unescaped Blade output) found — all output uses `{{ }}`.
- No LDAP or XPath injection vectors found.

---

### A04 — Insecure Design

#### [CRITICAL] A04-1: `billing_status` writable by org_admin via API

- **File:** `api/app/Http/Requests/Api/V1/UpdateOrganisationRequest.php:26`
- **File:** `api/app/Models/Organisation.php:23`
- **Description:** `billing_status` is in `Organisation::$fillable` AND in `UpdateOrganisationRequest` rules. Any org_admin can self-upgrade from `suspended`/`trial` to `active`.
- **Risk:** Billing bypass — organisations can override their subscription status.
- **Fix:** Remove `billing_status` from `UpdateOrganisationRequest::rules()`. Create a separate `super_admin`-only endpoint.

#### [CRITICAL] A04-2: No rate limiting on `auth/login` endpoint

- **File:** `api/routes/api.php:58`
- **Description:** `Route::post('auth/login', LoginController::class)` has no `throttle` middleware. Unlimited credential-guessing is possible.
- **Risk:** Full credential-stuffing and password-spray attacks.
- **Fix:** `->middleware('throttle:5,1')`. For tighter control, configure a named limiter keyed on `email|ip`.

#### [CRITICAL] A04-3: No rate limiting on `portal/token-auth` endpoint

- **File:** `api/routes/api.php:73`
- **Description:** No throttle middleware on `POST portal/token-auth`. Combined with ULID predictability (A02-1), token enumeration becomes practical.
- **Risk:** Automated token brute-force/enumeration.
- **Fix:** `->middleware('throttle:10,1')`.

#### [CRITICAL] A04-4: `portal/token-auth` returns raw model data including all columns

- **File:** `api/app/Http/Controllers/Api/V1/PortalTokenController.php:29-33,42-47`
- **Description:** Returns `(array) $artist` and `$event?->toArray()` — every database column exposed including internal fields, milestone booleans, project_leader_id, etc.
- **Risk:** Internal operational data exposed to portal users. Violates "Return all responses through API resources."
- **Fix:** Create `ArtistPortalResource` and `PortalEventResource` to whitelist only necessary fields.

#### [HIGH] A04-5: `Person::$fillable` includes `user_id` and `status`

- **File:** `api/app/Models/Person.php:27,36`
- **Description:** `user_id` (identity link) and `status` (approval state) are in `$fillable`. Controller layer is safe today, but model-level permission enables future mass-assignment.
- **Risk:** Latent mass-assignment risk for identity hijacking.
- **Fix:** Remove `user_id` from `$fillable`. Set explicitly: `$person->user_id = $user->id`.

#### [HIGH] A04-6: `ShiftAssignment::$fillable` includes audit trail fields

- **File:** `api/app/Models/ShiftAssignment.php:23-40`
- **Description:** `auto_approved`, `assigned_by`, `approved_by`, `approved_at`, `cancelled_by`, `cancelled_at`, `checked_in_at`, `checked_out_at` are fillable.
- **Risk:** Future `fill($request->all())` call could forge audit trails.
- **Fix:** Remove all audit-trail fields from `$fillable`. Set explicitly in the service layer.

#### [HIGH] A04-7: `UserInvitation::$fillable` includes role, token, status

- **File:** `api/app/Models/UserInvitation.php:18-27`
- **Description:** `role`, `token`, `status`, `organisation_id`, `invited_by_user_id`, `expires_at` are all fillable — entirely system-set fields.
- **Risk:** Latent mass-assignment vector.
- **Fix:** Remove from `$fillable`, set explicitly in `InvitationService`.

#### [MEDIUM] A04-8: No self-approval guard in `ShiftAssignmentPolicy`

- **File:** `api/app/Policies/ShiftAssignmentPolicy.php:19-23`
- **Description:** An org_admin who is also a registered volunteer can approve their own shift assignment.
- **Risk:** Self-approval of shift assignments bypasses the approval workflow.
- **Fix:** Add `if ($assignment->person?->user_id === $user->id) return false;` in `approve()`.

#### [MEDIUM] A04-9: No rate limiting on volunteer registration endpoint

- **File:** `api/routes/api.php:72`
- **Description:** `events/{event}/volunteer-register` has no throttle middleware.
- **Risk:** Automated mass registration / spam.
- **Fix:** `->middleware('throttle:5,1')`.

#### [LOW] A04-10: No rate limiting on invitation accept endpoint

- **File:** `api/routes/api.php:62`
- **Description:** `invitations/{token}/accept` has no throttle middleware.
- **Risk:** Token brute-force (mitigated by ULID entropy).
- **Fix:** `->middleware('throttle:10,1')`.

#### [LOW] A04-11: Inline `validate()` in `PasswordResetController`

- **File:** `api/app/Http/Controllers/Api/V1/PasswordResetController.php:28-32`
- **Description:** Both methods use `$request->validate()` inline instead of Form Requests. Violates project rules.
- **Risk:** Inconsistent validation patterns.
- **Fix:** Extract to dedicated Form Requests.

---

### A05 — Security Misconfiguration

#### [HIGH] A05-1: No security headers middleware

- **Description:** No `X-Content-Type-Options`, `X-Frame-Options`, `Strict-Transport-Security`, or `Content-Security-Policy` headers are added anywhere in the middleware stack.
- **Risk:** Clickjacking, MIME sniffing attacks, missing HSTS.
- **Fix:** Create and register a `SecurityHeaders` middleware with: `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Strict-Transport-Security: max-age=31536000; includeSubDomains`, `X-XSS-Protection: 1; mode=block`.

#### [MEDIUM] A05-2: `APP_DEBUG=true` in `.env.example`

- **File:** `api/.env.example:4`
- **Description:** `.env.example` defaults `APP_DEBUG=true`, training developers to leave it enabled. The exception handler (correctly) gates debug output on `config('app.debug')`.
- **Risk:** Accidental production deployment with debug enabled exposes stack traces and query details.
- **Fix:** Change `.env.example` to `APP_DEBUG=false` with comment `# Set to true only in local development`.

#### [MEDIUM] A05-3: Session `SameSite=Lax` may break cross-subdomain Sanctum cookie auth

- **File:** `api/config/session.php:202`
- **Description:** With two SPAs on different subdomains calling `api.crewli.app`, `SameSite=Lax` blocks session cookies on cross-origin requests.
- **Risk:** Sanctum SPA authentication may fail silently in production.
- **Fix:** Set `SESSION_DOMAIN=.crewli.app` and `SESSION_SAME_SITE=none` (with `SESSION_SECURE_COOKIE=true`) in production.

#### [MEDIUM] A05-4: `/mail-preview/{type}` web route exposes real database records without auth

- **File:** `api/routes/web.php:12-52`
- **Description:** Mail preview gated by `app()->environment('local', 'staging')` but fetches real Person, Event, UserInvitation records with no authentication.
- **Risk:** Staging environment exposes real data including invitation tokens.
- **Fix:** Add `->middleware('auth:sanctum')` or generate previews using factory-only data.

#### [MEDIUM] A05-5: Local `.env` uses wrong mail sender domain

- **File:** `api/.env:50`
- **Description:** `MAIL_FROM_ADDRESS="noreply@eventcrew.nl"` — should be `noreply@crewli.app` per CLAUDE.md.
- **Risk:** Failed SPF/DKIM, confused recipients.
- **Fix:** Update to `noreply@crewli.app`.

#### [LOW] A05-6: `allowed_headers: ['*']` in CORS config

- **File:** `api/config/cors.php:22`
- **Description:** Accepts any header from allowed origins. Only `Content-Type`, `Authorization`, `X-Requested-With`, `Accept` are needed.
- **Risk:** Overly broad attack surface.
- **Fix:** Constrain to needed headers.

#### Positive findings (A05)

- CORS `allowed_origins` correctly restricted to two specific SPAs (not `*`).
- `supports_credentials: true` correctly set for Sanctum.
- `APP_DEBUG` defaults to `false` in `config/app.php`.
- No Telescope/Horizon routes found (not installed).
- No phpinfo endpoints exposed.
- Password hashing uses bcrypt with `BCRYPT_ROUNDS=12`.
- `$password` is in `$hidden` on User model.

---

### A06 — Vulnerable Components

#### [CRITICAL] A06-1: `@casl/ability` 6.7.3 — Prototype Pollution

- **File:** `apps/app/package.json`, `apps/portal/package.json`
- **Description:** CASL is used for frontend permission enforcement across both apps. Prototype pollution could corrupt ability checks.
- **Risk:** Permission bypass in the frontend. Admin tokens at elevated risk.
- **Fix:** Upgrade to `>=6.7.5` in both apps.

#### [CRITICAL] A06-2: `axios` ~1.13.x — SSRF and metadata exfiltration

- **File:** `apps/app/package.json`, `apps/portal/package.json`
- **Description:** Two critical CVEs: `NO_PROXY` hostname normalization bypass (SSRF) and unrestricted cloud metadata exfiltration via header injection.
- **Risk:** Cloud metadata theft, SSRF in SSR contexts.
- **Fix:** Upgrade to `>=1.15.0` in both apps.

#### [CRITICAL] A06-3: `swiper` 11.2.10 — Prototype Pollution

- **File:** `apps/app/package.json`, `apps/portal/package.json`
- **Description:** Prototype pollution vulnerability.
- **Risk:** Client-side code execution.
- **Fix:** Upgrade to `>=12.1.2` (major version bump — review migration guide).

#### [MEDIUM] A06-4: `league/commonmark` 2.8.0 — HTML injection bypass

- **File:** `api/composer.json`
- **Description:** Two CVEs: `allowed_domains` bypass in embed extension (CVE-2026-33347) and `DisallowedRawHtml` bypass via whitespace (CVE-2026-30838).
- **Risk:** If user-supplied Markdown is rendered, raw HTML injection is possible.
- **Fix:** `composer update league/commonmark` to `>=2.8.2`.

#### [LOW] A06-5: `phpunit/phpunit` 11.5.46 — Unsafe deserialization (dev only)

- **File:** `api/composer.json`
- **Description:** CVE-2026-24765 — dev dependency only, not deployed.
- **Risk:** CI/CD pipeline risk only.
- **Fix:** `composer update phpunit/phpunit` to `>=11.5.50`.

#### [LOW] A06-6: `psy/psysh` 0.12.18 — Local privilege escalation

- **File:** `api/composer.json` (via `laravel/tinker`)
- **Description:** CVE-2026-25129 — local privilege escalation via CWD `.psysh.php` auto-load.
- **Risk:** Low — requires shell access. Confirm Tinker is not exposed via web route.
- **Fix:** `composer update laravel/tinker`.

#### [LOW] A06-7: Frontend dev dependencies — 45 vulnerabilities in build tools

- **File:** `apps/app/pnpm-lock.yaml`, `apps/portal/pnpm-lock.yaml`
- **Description:** `vite`, `rollup`, `tar`, `undici`, `minimatch`, `svgo`, `flatted`, `immutable`, `lodash`, `picomatch` — various high/moderate vulnerabilities in dev/build-time dependencies.
- **Risk:** Not shipped to production. CI/CD pipeline risk.
- **Fix:** Update dev dependencies: `pnpm update` for each app.

---

### A07 — Identification and Authentication Failures

#### [MEDIUM] A07-1: Account enumeration via `CheckEmailController`

- **File:** `api/app/Http/Controllers/Api/V1/CheckEmailController.php:16-18`
- **Description:** Returns `{"exists": true/false}` for any email. Rate limited to 10/min but still enables targeted enumeration.
- **Risk:** Confirms which email addresses are registered users.
- **Fix:** Consider removing this endpoint or requiring CAPTCHA. If needed for UX, apply stricter rate limiting.

#### [MEDIUM] A07-2: Existing Sanctum tokens not revoked on password reset

- **File:** `api/app/Http/Controllers/Api/V1/PasswordResetController.php:35-43`
- **Description:** `Password::reset()` updates the password but does not revoke existing tokens. An attacker with a stolen token retains access after reset.
- **Risk:** Password reset does not evict active attackers.
- **Fix:** Add `$user->tokens()->delete();` in the closure after saving the new password.

#### [MEDIUM] A07-3: Weak password validation (min:8 only, no complexity)

- **File:** `api/app/Http/Requests/Api/V1/LoginRequest.php:22`
- **File:** `api/app/Http/Requests/Api/V1/AcceptInvitationRequest.php:27`
- **File:** `api/app/Http/Controllers/Api/V1/PasswordResetController.php:32`
- **Description:** All password fields only enforce `min:8`. No mixed case, digits, symbols, or breach-check.
- **Risk:** Common/weak passwords accepted.
- **Fix:** Use `Password::min(8)->mixedCase()->numbers()` consistently. Consider `->uncompromised()`.

#### [MEDIUM] A07-4: No rate limiting on invitation routes

- **File:** `api/routes/api.php:61-62`
- **Description:** `GET/POST invitations/{token}` and `invitations/{token}/accept` have no throttle.
- **Risk:** Token brute-force and enumeration. `show` endpoint confirms token validity (404 vs. 200).
- **Fix:** `->middleware('throttle:10,1')` on both routes.

#### [LOW] A07-5: `Schema::hasTable()` runtime checks in `PortalTokenController`

- **File:** `api/app/Http/Controllers/Api/V1/PortalTokenController.php:22,37`
- **Description:** Schema introspection on every request. The 501 response reveals database schema info.
- **Risk:** Minor info disclosure + performance cost.
- **Fix:** Remove `Schema::hasTable()` guards; handle via migration, not runtime.

#### [LOW] A07-6: Email in password reset URL query string

- **File:** `api/app/Providers/AppServiceProvider.php:19-21`
- **Description:** Email address included as plaintext query parameter in the reset URL. Appears in browser history, server logs, referrer headers.
- **Risk:** Email address leakage.
- **Fix:** Omit email from URL; require user to enter it on the reset form.

---

### A08 — Software and Data Integrity

#### Positive findings (A08)

- CSRF intentionally disabled for stateless token-based API — appropriate architecture.
- File uploads validate `image`, `mimes:jpg,jpeg,png,webp`, `max:5120`. Uses server-generated filenames.
- All dependency lock files committed (`composer.lock`, `pnpm-lock.yaml` x3).
- Sanctum token prefix is empty (`''`) — consider setting `SANCTUM_TOKEN_PREFIX=crewli_` to enable GitHub secret scanning.

---

### A09 — Security Logging and Monitoring

#### [HIGH] A09-1: Failed login attempts not logged

- **File:** `api/app/Http/Controllers/Api/V1/LoginController.php:17`
- **Description:** On failed `Auth::attempt()`, returns 401 with no log entry, no event firing.
- **Risk:** Brute-force attacks leave no audit trail.
- **Fix:** Add `Log::warning('Failed login attempt', ['email' => ..., 'ip' => ..., 'user_agent' => ...])`.

#### [MEDIUM] A09-2: Authorization failures (403) not logged

- **File:** `api/bootstrap/app.php:65-90`
- **Description:** `AuthorizationException` is handled but not logged. Cross-tenant access probes leave no trace.
- **Risk:** No visibility into privilege escalation attempts.
- **Fix:** Add a `Log::warning()` in the exception handler for `AuthorizationException` with user, IP, path, method context.

#### [MEDIUM] A09-3: Missing activity log on 10+ critical actions

- **Description:** `spatie/laravel-activitylog` is installed but many critical actions are not logged:
  - Login success/failure
  - Logout
  - Person approved/rejected
  - Shift assignment approved/rejected/cancelled
  - Portal profile updated / password changed
  - Password reset completed
  - Organisation settings changed
  - Event status transitions
  - Identity match confirmed/dismissed
- **Risk:** No audit trail for administrative decisions.
- **Fix:** Add `activity()` or `Log::info()` calls for these actions.

#### [MEDIUM] A09-4: No suspicious pattern detection

- **Description:** No mechanism to detect rapid failed logins, cross-tenant access attempts, or token enumeration.
- **Risk:** Attacks proceed indefinitely without alerts.
- **Fix:** Configure `RateLimiter` in `AppServiceProvider::boot()` with `Lockout` notifications after N failures.

#### [LOW] A09-5: `VolunteerRegistrationService` catch block swallows exception context

- **File:** `api/app/Services/VolunteerRegistrationService.php:143-154`
- **Description:** `UniqueConstraintViolationException` is caught and re-thrown as `ValidationException` without logging the original.
- **Risk:** Race condition failures are invisible.
- **Fix:** Log the original exception before rethrowing.

---

### A10 — Server-Side Request Forgery

#### [LOW] A10-1: `email_logo_url` accepts any HTTP URL rendered in email templates

- **File:** `api/app/Http/Requests/Api/V1/UpdateOrganisationRequest.php:28`
- **File:** `api/resources/views/mail/layouts/crewli.blade.php:22`
- **Description:** User-supplied URL used as `<img src>` in outbound emails. Email clients request this URL, leaking recipient IP and metadata.
- **Risk:** Tracking pixel injection, recipient metadata exfiltration.
- **Fix:** Restrict to HTTPS only (`regex:/^https:\/\//`). Consider server-side image proxy.

#### Positive findings (A10)

- No `Http::get/post`, `file_get_contents()`, or `curl_*` calls in application code.
- No webhook or callback URL configurations found.
- No ZenderService or external HTTP client services exist.

---

### Frontend Security (A13)

#### ~~[CRITICAL] A13-1: Bearer tokens stored in `localStorage` (apps/app and apps/portal)~~ RESOLVED

- **File:** `apps/app/src/stores/useAuthStore.ts` (single SPA post WS-3)
- **Description:** Pre-WS-3 (April 2026) the SPA layer used per-app cookies (`crewli_app_token`, `crewli_portal_token`) with Origin-based middleware resolution. WS-3 PR-B consolidated the dual SPAs into a single `apps/app` workspace; PR-B2b retired the dual-cookie machinery. The system now issues a single httpOnly `crewli_app_token` cookie. The localStorage-based bearer-token storage that this finding originally flagged was migrated to httpOnly cookies as part of the same consolidation arc.
- **Resolution:** Tokens are httpOnly + Secure + SameSite=Strict, set server-side, never exposed to JavaScript. See `dev-docs/AUTH_ARCHITECTURE.md` for current architecture.

#### ~~[HIGH] A13-2: Admin app cookies lack `httpOnly`, `Secure`, and `SameSite` flags~~ RETIRED

- **File:** `apps/admin/` (removed)
- **Description:** The admin SPA has been retired. Its functionality now lives in `apps/app/` under `/platform/*` routes.
- **Resolution:** Finding no longer applicable — `apps/admin/` has been removed.

#### ~~[HIGH] A13-3: Open redirect vulnerability on post-login redirect (all apps)~~ RESOLVED by WS-3 PR-B2b

- **File:** `apps/app/src/utils/postLoginRedirect.ts` (single SPA post WS-3)
- **Description:** Login pages accepted `?to=` query parameter and redirected to it after login without validating it's a relative path.
- **Risk:** Phishing: `https://crewli.app/login?to=https://evil.com/steal`.
- **Resolution:** WS-3 PR-B2a introduced a minimum precaution (`startsWith('/') && !startsWith('//')`); WS-3 PR-B2b replaced it with full validation. The `isSafeRelativePath` helper in `apps/app/src/utils/postLoginRedirect.ts` now rejects empty input, non-`/`-prefixed paths, protocol-relative URLs, backslashes (browsers normalise `\` → `/`), ASCII control characters (`\x00`–`\x1F`, `\x7F`), and anything the URL constructor parses to a different origin than a synthetic invalid base. 16 vitest specs pin the contract.

#### ~~[HIGH] A13-4: `v-html` with API-sourced data in admin app (template pages)~~ RETIRED

- **File:** `apps/admin/` (removed)
- **Description:** The admin SPA has been retired. Its functionality now lives in `apps/app/` under `/platform/*` routes.
- **Resolution:** Finding no longer applicable — `apps/admin/` has been removed.

#### ~~[HIGH] A13-5: Admin router guard relies on JS-readable cookie without server validation~~ RETIRED

- **File:** `apps/admin/` (removed)
- **Description:** The admin SPA has been retired. Its functionality now lives in `apps/app/` under `/platform/*` routes.
- **Resolution:** Finding no longer applicable — `apps/admin/` has been removed.

#### [MEDIUM] A13-6: Server error messages forwarded to UI

- **File:** `apps/app/src/pages/login.vue:59-69`
- **File:** `apps/portal/src/pages/login.vue:51`
- **Description:** Raw `data.message` from API errors rendered in UI. Could expose internal details if backend misconfigured.
- **Risk:** Information disclosure (table names, file paths).
- **Fix:** Use safelist-based error mapping. The portal's `mapLoginErrorMessage` is a good pattern — extend it.

#### ~~[MEDIUM] A13-7: Admin `main.ts` mount-error handler uses `innerHTML` interpolation~~ RETIRED

- **File:** `apps/admin/` (removed)
- **Description:** The admin SPA has been retired. Its functionality now lives in `apps/app/` under `/platform/*` routes.
- **Resolution:** Finding no longer applicable — `apps/admin/` has been removed.

#### [MEDIUM] A13-8: Portal localStorage event state persists across session expiry

- **File:** `apps/portal/src/stores/usePortalStore.ts:9-34,86-94`
- **Description:** Event data and registration state stored in localStorage survives logout (partially cleared).
- **Risk:** Stale data from previous sessions could contaminate new sessions.
- **Fix:** Verify `reset()` is called on all logout paths including token expiry via 401 interceptor.

#### [LOW] A13-9: No Content Security Policy on any SPA — RESOLVED

- **File:** `apps/app/index.html`, `apps/portal/index.html`, `api/app/Http/Middleware/SecurityHeaders.php`, `deploy/nginx/`
- **Description:** ~~Neither app set a CSP meta tag or header.~~
- **Risk:** Injected scripts have unrestricted access.
- **Resolution:** API CSP enforced via `SecurityHeaders` middleware (`default-src 'none'; frame-ancestors 'none'`). SPA CSP configured via Nginx snippets (`deploy/nginx/csp-spa.conf`, `csp-portal.conf`). Dev CSP meta tags added to all `index.html` files for local testing. See `deploy/README.md` for rollout instructions.
- **WS-7 follow-up (mei 2026):** SPA `connect-src` whitelists the GlitchTip event-ingest endpoint as an explicit security control — dev `http://localhost:8200`, prod `https://monitoring.hausdesign.nl` (RFC-WS-7 §3.5). This restricts outgoing observability traffic to a single known host; without it, the strict CSP would either silently drop events (PR-3 regression) or — if loosened blindly — allow exfiltration to arbitrary hosts. Regression-guard: `tests/Feature/Security/CspConnectsToObservabilityTest.php` reads both the dev meta tag and the production nginx config and asserts the host is present.

#### [LOW] A13-10: No hardcoded secrets found in frontend code (positive)

- **Description:** No API keys, secrets, or credentials found in `.env.local` files or source code. `.env.local` files correctly gitignored.

---

## WS-7 Observability — finale audit (mei 2026)

### Acceptance criteria

Alle 14 criteria uit `RFC-WS-7-OBSERVABILITY.md §6` zijn geadresseerd:

| # | Criterium | Status |
|---|---|---|
| 1 | GlitchTip op `monitoring.hausdesign.nl` met TLS + 2FA | Bert-handmatig (deploy-checklist) |
| 2 | Twee projecten + DSNs in 1Password vault | Bert-handmatig (deploy-checklist) |
| 3 | Laravel SDK geïntegreerd; errors uit prod-API verschijnen <60s | ✅ PR-2 |
| 4 | apps/app SDK geïntegreerd; errors met org/user/release context; portal-routes strict scrub | ✅ PR-3 |
| 5 | Sourcemaps upload werkt; leesbare stack traces; `.map` afwezig in publieke bundle | ✅ PR-3 (`deploy.sh`) |
| 6 | PII scrubbing-tests groen (backend + frontend) | ✅ PR-2 + PR-3 |
| 7 | Smoke test induced 500 in staging | Bert-handmatig (deploy-checklist) |
| 8 | `ARCH-OBSERVABILITY.md` geschreven (WS-8b) | ✅ PR-4 |
| 9 | Email-alerting geconfigureerd + getest | Bert-handmatig (GlitchTip UI configuratie) |
| 10 | Retention-policy 90 dagen toegepast | Bert-handmatig (GlitchTip admin) |
| 11 | Daily postgres-backup-script in place | ✅ PR-1 |
| 12 | Activity_log indexes (D-06) gemigreerd | ✅ Pre-existing (Spatie default `nullableMorphs`); regression-guard `tests/Feature/Database/ActivityLogIndexesTest.php` |
| 13 | Structured logging conventie geïmplementeerd; `X-Request-Id` round-trip getest | ✅ PR-2 |
| 14 | `SECURITY_AUDIT.md` bijgewerkt | ✅ This entry |

### Processing register

GlitchTip is opgenomen in Crewli's processing register als zelfstandig
verwerkingsproces:

- **Doel:** defectdetectie en service-availability monitoring.
- **Categorieën persoonsgegevens:** pseudonieme identifiers (ULIDs voor
  user/organisation/event), technische metadata (route names, HTTP
  methods, stack traces zonder locals).
- **Bron:** geautomatiseerd captured uit Laravel API en apps/app SPA
  bij programmer/infra errors.
- **Ontvanger:** alleen Bert (single super_admin met 2FA op GlitchTip
  web-UI).
- **Bewaartermijn:** 90 dagen, daarna automatisch gepurged door
  GlitchTip's eigen partition-maintenance loop.
- **Beveiligingsmaatregelen:** TLS in transit, full-disk encryption at
  rest, SSH-key + 2FA op web-UI.
- **Controller / processor:** Crewli is **controller**. Self-hosted op
  Crewli-infra; geen processor-relatie of DPA-uitbreiding nodig.
- **Procedure right to erasure (Art. 17):** zie
  [`runbooks/observability-erasure.md`](./runbooks/observability-erasure.md).

### Security controls die WS-7 introduceert

1. **PII scrubbing op events (back + front).** `SentryEventScrubber`
   (PHP) en `scrubEvent` (TypeScript) strippen sensitive body-keys,
   headers, query-params, cookies, en form_values voordat events naar
   GlitchTip gestuurd worden. Regression-guards:
   `tests/Feature/Observability/PiiScrubbingTest.php` (20 cases) en
   `apps/app/src/observability/__tests__/scrubber.spec.ts` (18 cases).

2. **CSP `connect-src` whitelist voor named ingest host.** Outgoing
   Sentry-events zijn beperkt tot `localhost:8200` (dev) en
   `monitoring.hausdesign.nl` (prod). Geen exfiltration mogelijk naar
   arbitrary hosts. Regression-guard:
   `tests/Feature/Security/CspConnectsToObservabilityTest.php`. Zie
   ook A13-9 hierboven.

3. **Sourcemap upload-only, never public-mapped.** `deploy.sh` upload
   sourcemaps naar GlitchTip vóór `find apps/app/dist -name '*.map'
   -delete`. Stack-traces leesbaar in GlitchTip UI; geen `.map`
   bestanden bereikbaar via productie-bundle. Default soft-fail op
   upload faalt zodat deploy doorgaat (unmapped frames in GlitchTip
   is acceptabel; geblokkeerde deploy niet).

4. **Listener registration discipline.** Auto-discovery
   uitgeschakeld; alle observability listeners expliciet geregistreerd
   in `AppServiceProvider::boot()` met array-callable form.
   Regression-guard:
   `tests/Feature/Observability/EventListenerRegistrationTest.php`
   (BACKLOG OBS-8). Voorkomt silent double-emission die op een
   toekomstig moment additive operations zou breken.

5. **Runtime context-split portal/organizer.** Frontend portal-zone
   (`route.meta.public === true && route.meta.context === 'portal'`)
   krijgt geen `user_id` of `username` op events. RFC §3.7 frontend-
   block punt 5 — ULID-tokens voor token-based access (artist
   advance, public form fill) blijven uit GlitchTip-events.
   Regression-guard:
   `apps/app/src/observability/__tests__/contextBinding.spec.ts`
   (cross-zone leak test).

6. **Multi-tenant invariant op tag-niveau.**
   `actor_scope=organisation` impliceert valide ULID `organisation_id`;
   `actor_scope=platform` impliceert geen `organisation_id` (forced
   fallback zou misleidend zijn). Regression-guard:
   `AuthScopeContextListenerTest::test_organisation_id_present_when_actor_scope_is_organisation`.

7. **`impersonation.active` always-present binary signal.** Default-
   in-listener (`'false'`) + override-in-middleware (`'true'`) pattern
   garandeert dat élke authenticated event een binary signal voor
   filtering heeft. Regression-guard:
   `AuthScopeContextListenerTest::test_impersonation_active_default_false_across_every_actor_scope_branch`.

### Pointer naar Art. 17 procedure

GDPR right-to-erasure verzoeken voor GlitchTip-data: zie
[`runbooks/observability-erasure.md`](./runbooks/observability-erasure.md)
voor stap-voor-stap procedure. Geautomatiseerd erasure-script staat
op BACKLOG; tot dan handmatige psql-procedure met audit-trail
verplichting.

---

## Positive Findings

The following security measures ARE correctly implemented:

1. **Password hashing:** bcrypt with `BCRYPT_ROUNDS=12`. `$password` in User model's `$hidden` array.
2. **CORS origins:** Correctly restricted to two specific SPA origins (not `*`). `supports_credentials: true` set.
3. **SQL injection prevention:** All Eloquent queries use PDO parameter binding. All `whereRaw()`/`selectRaw()` calls use `?` placeholders.
4. **No shell execution:** No `exec()`, `shell_exec()`, `system()`, `proc_open()`, or `passthru()` in application code.
5. **Blade escaping:** All Blade output uses `{{ }}` (escaped). Zero instances of `{!! !!}`.
6. **CSRF:** Intentionally disabled for stateless token-based API — correct architecture.
7. **File uploads:** Validated for type, mime, and size. Server-generated filenames prevent path traversal.
8. **Lock files:** `composer.lock` and all `pnpm-lock.yaml` files committed.
9. **Password reset throttling:** `auth/forgot-password` has `throttle:5,1`.
10. **Check-email throttling:** `public/check-email` has `throttle:10,1`.
11. **Debug output gated:** Exception handler correctly gates debug info behind `config('app.debug')`.
12. **No Telescope/Horizon exposed:** Not installed in the application.
13. **Invitation expiry:** Invitations expire after 7 days and are checked for `pending` status + `expires_at`.
14. **Activity logging:** `spatie/laravel-activitylog` in use for invitation and volunteer registration flows.
15. **Policy coverage:** 16 policies exist covering all major models. Controllers consistently use `Gate::authorize()`.
16. **No external HTTP calls:** No outbound HTTP client usage in application code — no SSRF surface.

---

## Priority Remediation Plan

### Immediate (before any production deployment)

| Priority | Finding | Fix effort |
|----------|---------|-----------|
| 1 | A04-2: Rate limit login | One line |
| 2 | A04-3: Rate limit portal/token-auth | One line |
| 3 | A04-4: API resources for portal/token-auth response | Small |
| 4 | A04-1: Remove billing_status from UpdateOrganisationRequest | One line |
| 5 | A02-2: Set Sanctum token expiration | One line |
| 6 | A02-1: Replace ULID tokens with cryptographic random | Small |
| 7 | A01-1: Implement PortalTokenMiddleware | Medium |
| 8 | ~~A13-3: Fix open redirect on login pages~~ ✅ resolved by WS-3 PR-B2b | Small |

### Short-term (within 1 sprint)

| Priority | Finding | Fix effort |
|----------|---------|-----------|
| 9 | A01-2/3: Add event boundary checks in portal shift + policy | Small |
| 10 | A01-5/6/7/8/9: Scope all `exists:` rules to org/event | Medium |
| 11 | A01-4: Register OrganisationScope on models | Medium |
| 12 | A04-5/6/7: Clean up $fillable on Person, ShiftAssignment, UserInvitation | Small |
| 13 | A07-2: Revoke tokens on password reset | One line |
| 14 | A05-1: Add security headers middleware | Small |
| 15 | A06-1/2: Upgrade @casl/ability and axios | Small |
| 16 | A13-1: Migrate token storage to httpOnly cookies | Medium |

### Medium-term (within 2-3 sprints)

| Priority | Finding | Fix effort |
|----------|---------|-----------|
| 17 | A01-13: Move event routes under org prefix | Large |
| 18 | A09-1/2/3: Add security logging | Medium |
| 19 | A07-3: Strengthen password rules | Small |
| 20 | ~~A13-2/5: Fix admin cookie security~~ RETIRED (apps/admin removed) | N/A |
| 21 | A06-3: Upgrade swiper (major version) | Medium |