# Security Audit Report — product-db (vehicle catalog)

Date: 2026-08-11 · Scope: `/mnt/devel/md/product-db` (Laravel 13 + Inertia v3 + React 19)
Method: static code analysis (app/, routes/, config/, resources/js/, tests/), env/config inspection, live DB read-only queries. No business-scope changes; only 2 minimal security fixes applied (documented at the end).

---

## 1. SQL Injection — CLEAN (no findings)

Every path was traced; all user input reaches SQL only through the query builder (parameter binding) and all raw SQL uses hardcoded identifiers.

- **Catalog query services** — all parameterized. Sort **columns** are whitelisted via constants + `match` (never concatenated): `ModelsQuery::SORTS` (ModelsQuery.php:14), `applySort` match (ModelsQuery.php:71-81); `VariantsQuery::SORTS` (VariantsQuery.php:15, applySort 90-108). Sort **direction** whitelisted: `match (strtolower(...)) { 'desc' => 'desc', default => 'asc' }` (ModelsQuery.php:65-70, VariantsQuery.php:84-89). Search/filters all `where('col', 'like', "%{$search}%")` — parameterized (ModelsQuery.php:39-43, VariantsQuery.php:48-57, StructureQuery.php:60-64). `CatalogMetrics::totals()` interpolates `self::COUNT_TABLES` — hardcoded class constant (CatalogMetrics.php:33-41). `CatalogStatusCounts::for(string $table)` — table names passed from controllers as literal strings (`'models'`, `'variants'`), not user input.
- **PcmMigration** — `SourceReader::stream()` builds ``SELECT `cols` FROM `table` ORDER BY `cols[0]` `` (SourceReader.php:213); all callers pass column lists from class constants / hardcoded arrays (Migrator `MODEL_COLUMNS` etc.). `localeKeyPresence` / `spotCheckLocaleValues` interpolate `$column` (PHPDoc `literal-string`, values from `LOCALIZED_COLUMNS` / `JSON_COLUMNS` constants) and `$key` from the hardcoded locale map `['de-CH','fr-CH','it-CH']` (SourceReader.php:461-465, Verifier.php:758-763). `Verifier::checkOrphans` SQL strings are 100% hardcoded literals (Verifier.php:988-1009). `Migrator::upsert` builds column identifiers from constants and binds all VALUES as `?` placeholders via `->statement($sql, $values)` (Migrator.php:1028-1048).
- **No `DB::selectRaw`/`whereRaw` consumes request input anywhere in the app.** `php artisan route:list` confirms catalog endpoints are the only request surface.

## 2. Mass Assignment — CLEAN

All models declare explicit `#[Fillable([...])]` attribute lists (Laravel 13 attributes): `VehicleModel` (VehicleModel.php:35), `Variant`, `StructureElement`, `User` (User.php:36, with `#[Hidden([...])]` for `password`, `two_factor_secret`, `two_factor_recovery_codes`, `remember_token`). The only request-driven write is `ProfileController::update` which uses `fill($request->validated())` on a whitelisted FormRequest (ProfileController.php:38-46). Migrator writes go through explicit column arrays (see §1). No `$request->all()` into `create/update` anywhere.

## 3. Validation — GOOD

- `ModelsIndexRequest` / `VariantsIndexRequest`: `Rule::exists` on `brands.code`, `markets.code`, `models.id`, `body_types.id`, `engines.id`, `trims.id`; `status` → `Rule::in(CatalogStatus::ALL)`; `sort` → `Rule::in(SORTS)`; `direction` → `in:asc,desc`; `per_page` → `integer between:1,100` (ModelsIndexRequest.php, VariantsIndexRequest.php). `StructureIndexRequest`: brand exists, `element_type` in whitelist, per_page 1-50.
- Unknown filter values → validation failure (422 / redirect back), **not 500**. `ImportsController` clamps `per_page` inline to 1–100 (ImportsController.php:22).
- **INFO:** `ImportsController` uses no FormRequest (inline clamp is adequate).

## 4. IDOR / Data Exposure — LOW

- Catalog is **intentionally public** (documented in routes/web.php:20-23 and AGENTS.md) — accepted risk, no auth invented.
- **`salesforce_id_json` is NOT serialized anywhere** — verified across all 7 resources + `BodyTypeDetailQuery`; it exists only in the DB and in Verifier count checks. Good.
- **LOW:** `pcm_id` (vendor surrogate ids) is exposed in every resource (VehicleModelResource.php:20, VehicleModelDetailResource, VariantResource, VariantDetailResource, StructureCategoryResource) and `sap_code` (ERP codes) in body-type section payloads (BodyTypeDetailQuery.php:64); `range_json` on body types (VehicleModelDetailResource). Product-catalog internal identifiers — low sensitivity, but reconsider if the catalog ever becomes customer-facing.
- **INFO:** `ImportRunResource` exposes import `notes` (data-quality notes + counts) on the public dashboard — consistent with the documented provenance feature; no secrets in notes (verified sample in pcm_imports).
- **Inertia shared props** (HandleInertiaRequests.php:33-45): only `name`, `auth.user` (null for guests), `sidebarOpen` — no leaks.

## 5. XSS — LOW (1 hardening fix applied)

- React escapes all props; single `dangerouslySetInnerHTML` at two-factor-setup-modal.tsx:82 renders the **server-generated Fortify QR SVG** (trusted source) — acceptable, INFO.
- **MED→FIXED:** `resources/views/app.blade.php:10` interpolates the attacker-controllable `appearance` cookie (excluded from cookie encryption, bootstrap/app.php:31) into an inline `<script>`: `const appearance = '{{ $appearance }}'`. Blade `{{ }}` escaping currently neutralizes both `</script>` breakout and string termination, but the value is fully attacker-controlled. **Fix applied:** `HandleAppearance` now whitelists `['light','dark','system']` and falls back to `'system'` (HandleAppearance.php:16-27).
- `sidebar_state` cookie → only boolean comparison (HandleInertiaRequests.php:44-45); the `appearance` cookie is the only cookie reaching an inline script.

## 6. CSRF — CLEAN

All state-changing routes are inside the `web` middleware group (CSRF enforced): settings PATCH/PUT/DELETE (routes/settings.php, standard Laravel), Fortify auth routes (login/register/2FA/passkeys) with standard CSRF; password change throttled `throttle:6,1`, security page behind `RequirePassword`, login/two-factor/passkeys rate-limited (FortifyServiceProvider.php:73-91). Catalog is GET-only. No `withoutMiddleware`/CSRF exceptions anywhere.

## 7. Secrets — MED (1 fix applied)

- **MED→FIXED:** `config/database.php` had a hardcoded credential fallback `'password' => env('PCM_DB_PASSWORD', 'DEVEL!now1')` — the PCM DB password committed in the repo tree. **Fix applied:** now `env('PCM_DB_PASSWORD')` with no fallback (fails closed); `PCM_DB_*` vars added to `.env` (gitignored) and as empty placeholders to `.env.example`.
- **INFO:** `.env` (with `DB_PASSWORD=DEVEL!now1`, `APP_KEY`, `APP_DEBUG=true`) is listed in `.gitignore` (line 15); the directory currently has **no .git repo**, so nothing is committed — but if VCS is initialized, `.gitignore` protects it. `.env.example` carries no secrets.
- **Legacy (out of repo scope, noted):** prod credentials in legacy `cockpit/configuration.php`, digitas/atlassian tokens in legacy configs — documented in existing-system.md, not part of this repo; recommend rotation/removal of the legacy copies.

## 8. Session / Cookies — LOW

- `SESSION_DRIVER=database` (session.php:21); `http_only` default true (185); `SameSite=lax` (202); `secure` = `env('SESSION_SECURE_COOKIE')` → **unset in .env** — session cookie travels over plain HTTP in dev (acceptable locally; must set `SESSION_SECURE_COOKIE=true` + HTTPS `APP_URL` before any real deployment). LOW.
- `appearance`/`sidebar_state` excluded from encryption by design (non-sensitive, now whitelisted server-side — see §5).

## 9. DoS / Heavy Queries — LOW

- `per_page` bounded on every endpoint (1–100 / 1–50, validated or clamped).
- **LOW:** LIKE wildcards `%`/`_` from `search` are not escaped (parameterized, but leading `%` defeats index use; bounded by `max:255`). Options: escape wildcards or use a match index.
- **INFO:** `/variants` loads the full option lists (all models/body_types/engines/trims ≈ 2.3k rows) per request; model detail eager-loads the whole hierarchy + deferred feature-preview aggregates (bounded by catalog size; 69k-row trim feature scan is deferred but still computed server-side). No rate limiting on public catalog endpoints — a throttle (`throttle:60,1` or similar) is cheap hardening once auth lands.

## 10. Error Handling — LOW

- `APP_DEBUG=true`, `APP_ENV=local` in `.env` — dev-only; **must not ship to production** (stack traces would leak). `.env.example` also defaults `APP_DEBUG=true` (starter-kit default) — set false in prod env. LOW.
- Exceptions config renders JSON for `api/*` + Inertia requests (bootstrap/app.php:44-47) — standard, no detail leakage beyond Laravel defaults. Closure routes use `abort_unless(..., 404)` for out-of-hierarchy ids (routes/web.php:66-90); implicit model binding 404s otherwise. GOOD.

## 11. Logging — CLEAN

- `Log::info('pcm import run completed', ['run_id', 'rows'])`, `Log::error('pcm import run failed', ['error' => message])`, summary logs (Migrator.php). `pcm_imports.notes` holds data-quality notes + counts only (verified live). No credentials/passwords/tokens logged anywhere (`grep` over app/ for `password|secret|token` log context).

---

## Fixes applied (2, minimal)

1. **HandleAppearance whitelist** (app/Http/Middleware/HandleAppearance.php) — cookie value restricted to `light|dark|system` before reaching the inline script in app.blade.php. LOW severity hardening.
2. **PCM DB password fallback removed** (config/database.php) — credential now env-only, fail-closed; `.env`/`.env.example` updated. MED severity.

Both are behavior-preserving (same credential via .env; same appearance modes).

## Gates

- `php artisan test --compact` → **124 passed / 124, 1501 assertions** (final full run). ⚠ Earlier runs were flaky — root cause found: a leftover debug test `tests/Feature/DebugOrderTest.php` (created 2026-08-11 01:20, always passes, dumps output) **truncates the live product_db and re-runs the full migration**, racing `CatalogRealDataTest` / `ProductDbMigrateCommandTest` which share the same live MySQL. Failures were random (3-vs-2 run counts, empty-table reads), all unrelated to the fixes. **Recommendation: delete DebugOrderTest.php (with approval) and/or give the real-data tests a dedicated schema.** Rule recorded in `.ai/rules/feature.md`.
- `PHPSTAN_TURBO=0 php vendor/bin/phpstan analyse --memory-limit=1G` → **0 errors** (level 7).
- `vendor/bin/pint` on changed files → passed.

## Findings summary

| # | Severity | Finding | Status |
|---|----------|---------|--------|
| 1 | MED | Hardcoded PCM DB credential fallback in config/database.php | **FIXED** |
| 2 | MED | Attacker-controlled cookie → inline script in app.blade.php | **FIXED** (whitelist) |
| 3 | LOW | `SESSION_SECURE_COOKIE` unset; HTTP cookies in dev | Report only |
| 4 | LOW | LIKE wildcards unescaped; heavy option-list queries; no rate limit on public GETs | Report only |
| 5 | LOW | `pcm_id`/`sap_code`/`range_json` exposed on public endpoints | Report only |
| 6 | LOW | `APP_DEBUG=true` defaults must not reach prod | Report only |
| 7 | MED | Flaky suite: DebugOrderTest truncates live DB, races other tests | Report only (approval needed to delete) |
| 8 | INFO | Legacy cockpit prod creds / digitas / atlassian tokens (out of repo) | Report only |

## Verdict

**ACCEPTABLE** — 0 unresolved HIGH findings, 0 unresolved MED findings. The two MED issues found were fixed; remaining items are LOW/INFO hardening notes and one test-hygiene item requiring approval. SQL-injection surface is clean (parameterized everywhere, identifiers whitelisted/constant), mass assignment is closed via `#[Fillable]`, catalog exposure is the documented accepted state, and auth/CSRF flows are stock Laravel. Before any production deployment: set `APP_DEBUG=false`, `SESSION_SECURE_COOKIE=true`, HTTPS `APP_URL`, and a real `APP_KEY`.
