# Review PR `feat/audit-log` — 4e passe

> Audit complet de la PR audit-log (89 commits, 233 fichiers, +52k/-876 lignes) apres les 3 passes de review deja mergees.
> Objectif : faire sortir ce qui reste avant merge dans `main`.
> Genere le 2026-04-23.

**Branche** : `feat/audit-log`
**Base** : `main`
**Revues anterieures** (deja appliquees dans la branche) :
- `bb6a4c3 fix(review) : blockers review PR #9`
- `25cd6a1 fix(review) : regression drawers RBAC + race snapshot + stale-data admin`
- `b1255bb fix(review) : 3e passe review (HIGH frontend + MEDIUMs)`
- `7117744 docs(claude) : refactor CLAUDE.md`

La branche est globalement solide : les trois miroirs RBAC sont synchronises, le pattern swap-and-clear de l'audit est correctement implemente, la connexion DBAL dediee est bien configuree. Les findings ci-dessous sont incrementaux et ne remettent pas en cause la feature.

---

## Table des matieres

1. [Securite](#1-securite)
2. [Bugs silencieux](#2-bugs-silencieux)
3. [Violations des regles projet](#3-violations-des-regles-projet)
4. [Incoherences de patterns](#4-incoherences-de-patterns)
5. [Documentation et configuration](#5-documentation-et-configuration)
6. [Frontend et UX](#6-frontend-et-ux)
7. [Bonnes pratiques a retenir](#7-bonnes-pratiques-a-retenir)
8. [Resume par priorite](#8-resume-par-priorite)

---

## 1. Securite

### 1.1 CRITIQUE — `/api/docs` public en production

**Fichier** : `config/packages/security.yaml:46`

```yaml
- { path: ^/api/docs, roles: PUBLIC_ACCESS }
```

La documentation Swagger/OpenAPI d'API Platform est accessible sans authentification, quel que soit l'environnement — y compris en production sur `starseed.malio-dev.fr`. Elle expose :

- la liste complete des endpoints (`/api/audit-logs`, `/api/users/{id}/rbac`, `/api/sites`, etc.)
- les schemas de securite (`is_granted('core.audit_log.view')`)
- les filtres acceptes par chaque provider (y compris `performed_at[after]`)
- la structure des DTOs (`AuditLogOutput`, `UserOutput`...)
- les patterns UUID/IDs

**Pourquoi c'est grave** : un attaquant a une cartographie gratuite de la surface d'attaque. Pour un CRM interne sur DNS public, c'est une fuite d'information inutile. API Platform genere cette doc automatiquement mais rien n'oblige a la rendre publique.

**Correction** : fermer en prod.

```yaml
# config/packages/security.yaml
- { path: ^/login_check, roles: PUBLIC_ACCESS }
- { path: ^/api/version, roles: PUBLIC_ACCESS, methods: [GET] }
- { path: ^/api/modules, roles: PUBLIC_ACCESS, methods: [GET] }
- { path: ^/api/sidebar, roles: PUBLIC_ACCESS, methods: [GET] }
- { path: ^/api, roles: IS_AUTHENTICATED_FULLY }
# supprimer la ligne "- { path: ^/api/docs, roles: PUBLIC_ACCESS }"
```

Ou conditionner l'acces au debug mode :

```yaml
when@prod:
    security:
        access_control:
            - { path: ^/api/docs, roles: IS_AUTHENTICATED_FULLY }
```

---

### 1.2 CRITIQUE — Aucun en-tete de securite HTTP en production

**Fichier** : `infra/prod/nginx.conf` et `infra/prod/nginx-proxy.conf` (aucune directive `add_header`)

Le Nginx de prod n'emet aucun des en-tetes de securite standards :

| En-tete | Role | Present ? |
|---------|------|-----------|
| `X-Frame-Options: DENY` | anti-clickjacking (pas d'embed iframe) | non |
| `X-Content-Type-Options: nosniff` | anti MIME-sniffing | non |
| `Referrer-Policy` | limite les fuites dans le Referer | non |
| `Content-Security-Policy` | anti-XSS | non |
| `Strict-Transport-Security` | force HTTPS | non |

Le reverse proxy ecoute uniquement sur le port 80 (HTTP), sans redirection 301 vers HTTPS. Combine avec `JWT_COOKIE_SECURE=1` (defaut dans `.env.prod.example`), le cookie ne serait meme pas envoye en HTTP — donc un premier acces HTTP casse le login silencieusement, l'utilisateur croira que l'auth est buggee.

**Correction minimale dans `nginx-proxy.conf`** (niveau proxy public) :

```nginx
server {
    listen 80;
    listen [::]:80;
    server_name starseed.malio-dev.fr;

    # Redirection HTTPS obligatoire (ajouter un server block HTTPS par ailleurs).
    # Tant que le TLS n'est pas en place, au minimum poser les en-tetes suivants.
    add_header X-Frame-Options "DENY" always;
    add_header X-Content-Type-Options "nosniff" always;
    add_header Referrer-Policy "strict-origin-when-cross-origin" always;

    # ... reste de la config
}
```

Quand TLS est en place, ajouter :

```nginx
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;
```

---

### 1.3 CRITIQUE — `robots.txt` autorise toute l'indexation

**Fichier** : `frontend/public/robots.txt`

```
User-Agent: *
Disallow:
```

La valeur `Disallow:` (vide) signifie "rien n'est interdit" — tous les crawlers peuvent indexer la totalite du site. Pour un outil CRM interne accessible sur un DNS public (`starseed.malio-dev.fr`), c'est un leak inutile : la page de login, les URLs `/admin/*`, les URLs des fiches clients peuvent remonter dans Google.

**Correction** :

```
User-Agent: *
Disallow: /
```

---

### 1.4 IMPORTANT — `performed_at[after|before]` sans typage DBAL → crash 500 sur date malformee

**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:182-186`

```php
if (isset($filters['performed_at_after'])) {
    $qb->andWhere('performed_at >= :performed_at_after')
       ->setParameter('performed_at_after', $filters['performed_at_after']);
}
```

La valeur est passee comme chaine brute a DBAL. La colonne `performed_at` est un `timestamptz`. Si un client envoie `?performed_at[after]=not-a-date`, PostgreSQL leve une erreur de cast et l'API retourne une 500. Pas d'injection SQL (le parametre est bien binde), mais :

- erreur 500 loguee pour chaque mauvaise entree (pollution des logs + bruit pour l'oncall)
- DoS tres bas effort : un utilisateur avec `core.audit_log.view` peut envoyer des requetes mal formees en boucle
- mauvaise UX : le front recoit une erreur generique au lieu d'un 400 explicite

**Correction** :

```php
use Doctrine\DBAL\Types\Types;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;

if (isset($filters['performed_at_after'])) {
    try {
        $after = new \DateTimeImmutable($filters['performed_at_after']);
    } catch (\Throwable) {
        throw new BadRequestHttpException('performed_at[after] doit etre une date ISO 8601 valide.');
    }
    $qb->andWhere('performed_at >= :performed_at_after')
       ->setParameter('performed_at_after', $after, Types::DATETIMETZ_IMMUTABLE);
}
// idem pour performed_at_before
```

Cette correction donne en prime un 400 propre avec un message clair.

---

### 1.5 IMPORTANT — Clause `ESCAPE` absente du `ILIKE` (filtre `performed_by`)

**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:177-180`

```php
$escaped = str_replace(['\\', '%', '_'], ['\\\\', '\%', '\_'], $filters['performed_by']);
$qb->andWhere('performed_by ILIKE :performed_by')
   ->setParameter('performed_by', '%'.$escaped.'%');
```

Le commentaire dit : *"`\` est deja le caractere d'echappement LIKE par defaut en PostgreSQL"*. C'est **inexact**. En SQL-standard PostgreSQL, il n'y a pas de caractere d'echappement par defaut pour LIKE/ILIKE : pour echapper `%` ou `_`, il faut soit `LIKE pattern ESCAPE '\'`, soit utiliser un autre caractere (ex: `ESCAPE '|'`).

En pratique, sur PostgreSQL avec `standard_conforming_strings=on` (defaut depuis 9.1), `\` n'est PAS interprete par LIKE. Donc `'%\_%'` matche la chaine `%\_%` — pas ce qu'on veut. Le filtre est silencieusement casse pour tout nom contenant `_` ou `%`.

**Test a faire en psql pour confirmer** :
```sql
SELECT 'admin_backup' ILIKE '%admin\_backup%';           -- t sur PG moderne ? non : f
SELECT 'admin_backup' ILIKE '%admin\_backup%' ESCAPE '\'; -- t
```

**Correction** : ajouter explicitement la clause `ESCAPE`.

```php
$qb->andWhere("performed_by ILIKE :performed_by ESCAPE '\\\\'")
   ->setParameter('performed_by', '%'.$escaped.'%');
```

(Les quatre `\` en PHP donnent deux `\` dans le SQL, soit un `\` litteral une fois parse par PostgreSQL.)

Alternative plus sure : utiliser `position()` au lieu de LIKE.

```php
$qb->andWhere('position(lower(:performed_by) IN lower(performed_by)) > 0')
   ->setParameter('performed_by', $filters['performed_by']);
```

---

### 1.6 IMPORTANT — `SiteAwareInjectionProcessor` : bypass silencieux si l'appelant n'est pas une instance de `User`

**Fichier** : `src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php:64-75`

```php
if (!$this->security->isGranted('sites.bypass_scope')) {
    $user = $this->security->getUser();
    $explicitSite = $data->getSite();
    if ($user instanceof User && $explicitSite instanceof Site && !$user->hasSite($explicitSite)) {
        throw new AccessDeniedHttpException(...);
    }
}
```

Si `$user` n'est pas exactement une instance de `App\Module\Core\Domain\Entity\User` (ex: futur provider d'auth tiers, token systeme), la condition `instanceof User` est fausse et la garde cross-site write est **silencieusement sautee**. L'utilisateur peut alors specifier n'importe quel `site` dans le payload sans verification.

Aujourd'hui le risque est faible (un seul `app_user_provider` configure). Mais le pattern est fragile : une absence de type doit lever une erreur, pas passer.

**Correction** : transformer le cas "pas un User" en refus explicite.

```php
if (!$this->security->isGranted('sites.bypass_scope')) {
    $user = $this->security->getUser();
    if (!$user instanceof User) {
        throw new AccessDeniedHttpException('Utilisateur non reconnu pour la validation de site.');
    }
    $explicitSite = $data->getSite();
    if ($explicitSite instanceof Site && !$user->hasSite($explicitSite)) {
        throw new AccessDeniedHttpException(...);
    }
}
```

---

### 1.7 IMPORTANT — `isHandlingUnauthorized` sans `try/finally` : flag bloque si `navigateTo` throw

**Fichier** : `frontend/shared/composables/useApi.ts:25,125-130`

```typescript
let isHandlingUnauthorized = false  // module-level singleton

// ...
if (!isLoginCheck && !isLogout) {
    if (!isHandlingUnauthorized) {
        isHandlingUnauthorized = true
        auth.clearSession()
        await navigateTo('/login')
        isHandlingUnauthorized = false  // <-- jamais atteint si navigateTo throw
    }
}
```

Si `navigateTo('/login')` echoue (middleware qui throw, plugin qui throw dans un hook, navigation cancelee par `abortNavigation`), le flag reste `true` **indefiniment**. Toutes les 401 futures sont silencieusement ignorees, l'utilisateur reste sur la page courante avec l'impression que les requetes ne font rien. Le seul remede est un hard-reload.

**Correction** : `try/finally`.

```typescript
if (!isLoginCheck && !isLogout) {
    if (!isHandlingUnauthorized) {
        isHandlingUnauthorized = true
        try {
            auth.clearSession()
            await navigateTo('/login')
        } finally {
            isHandlingUnauthorized = false
        }
    }
}
```

---

### 1.8 IMPORTANT — Pagination maximale absente sur `Permission`, `Role`, `Site` (itemsPerPage 999 cote front)

**Fichiers** :
- `frontend/modules/core/components/UserRbacDrawer.vue:235,236`
- `frontend/modules/core/components/RoleDrawer.vue:149`
- `frontend/modules/sites/pages/admin/sites.vue:117`

```typescript
api.get<{ member: Permission[] }>('/permissions', { orphan: false, itemsPerPage: 999 }, ...)
api.get<{ member: Site[] }>('/sites', { itemsPerPage: 999 }, ...)
```

Deux problemes cumules :

1. **`paginationClientItemsPerPage` n'est pas active** sur les resources `Permission`, `Role`, `Site` (seul `AuditLogResource` l'active). API Platform ignore donc `itemsPerPage=999` et retourne 30 elements par defaut. **Le `999` est un no-op**. Aujourd'hui ca marche parce que ces catalogues comptent <30 entrees, mais quand les modules grandiront, les drawers vont silencieusement tronquer.

2. **Aucun `paginationMaximumItemsPerPage`** n'est pose sur ces ressources. Si un dev decide d'activer `paginationClientItemsPerPage: true` plus tard, `?itemsPerPage=99999` deviendra une requete valide qui pourra faire suer la DB.

**Correction** : deux options selon l'intention.

*Option A — Desactiver la pagination pour ces catalogues* (ils sont small + exhaustifs par nature) :

```php
// Permission.php — GetCollection
new GetCollection(
    normalizationContext: ['groups' => ['permission:read']],
    security: "...",
    paginationEnabled: false,
),
```

Cote front, retirer `itemsPerPage: 999` (devient inutile).

*Option B — Garder la pagination avec un plafond explicite* :

```php
new GetCollection(
    paginationClientItemsPerPage: true,
    paginationMaximumItemsPerPage: 200,
    // ...
),
```

---

## 2. Bugs silencieux

### 2.1 IMPORTANT — `AuditLogDetail.vue` : `JSON.stringify` sans garde sur valeur non-serialisable

**Fichier** : `frontend/shared/components/audit/AuditLogDetail.vue` (fonction `formatValue`)

Si une valeur de `changes` est non-serialisable (objet circulaire, symbol, bigint), `JSON.stringify` throw et casse tout le rendu du drawer. Ce cas est theoriquement impossible avec les donnees ecrites par `AuditListener` aujourd'hui, mais un futur enrichissement (ex: serialisation d'un objet metier complexe) peut introduire ce risque.

**Correction** : wrapper defensif.

```typescript
function formatValue(value: unknown): string {
    if (value === null || value === undefined) return 'vide'
    if (typeof value === 'boolean') return value ? t('common.yes') : t('common.no')
    if (typeof value === 'object') {
        try { return JSON.stringify(value) } catch { return '[valeur non serialisable]' }
    }
    return String(value)
}
```

---

### 2.2 MOYEN — `UserRbacProcessor` : payload JSON invalide = regression silencieuse des collections

**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php:241-248`

Le processor parse `$request->getContent()` via `json_decode()` pour savoir quelles cles sont absentes du payload et restaurer les collections qu'API Platform aurait ecrasees. Si le body est un JSON invalide (rare mais possible : content-type incorrect, body vide suite a un intercepteur buggue), `json_decode` retourne `null` et la restauration est `return` sans aucun log.

Consequence : les collections `rbacRoles`, `directPermissions`, `sites` peuvent etre ecrasees par des tableaux vides sans trace. Bug quasi-impossible a diagnostiquer en prod.

**Correction** : logger `warning` dans ce cas.

```php
$payload = json_decode($request->getContent(), true);
if (!is_array($payload)) {
    $this->logger->warning('UserRbacProcessor : body JSON invalide, skip de restoreAbsentCollections', [
        'user_id' => $data->getId(),
    ]);
    return;
}
```

---

## 3. Violations des regles projet

### 3.1 MOYEN — `<button>` brut au lieu de `MalioButton`

**Fichier** : `frontend/shared/components/audit/AuditTimeline.vue:80`

```html
<button
    type="button"
    class="mt-3 text-sm text-blue-600 hover:text-blue-800"
    @click="loadMore"
>
    {{ t('audit.timeline.load_more') }}
</button>
```

**Regle violee** : `.claude/rules/frontend.md` — *"Tout champ de formulaire / filtre / bouton doit utiliser les composants Malio*"*.

C'est le seul bouton HTML brut dans la PR. Aucun commentaire `TODO` ne documente une exception.

**Correction** : utiliser `MalioButton` avec un variant secondaire/link.

```html
<MalioButton
    type="secondary"
    size="sm"
    :label="t('audit.timeline.load_more')"
    class="mt-3"
    @click="loadMore"
/>
```

Si `MalioButton` ne propose pas de variant "link" adapte, commenter l'exception :

```html
<!-- TODO(malio-ui) : MalioButton n'a pas encore de variant 'link-inline' -->
<button ...>
```

---

### 3.2 MOYEN — Cle i18n `sidebar.core.sites` sous le mauvais namespace

**Fichiers** :
- `config/sidebar.php:82` : `'label' => 'sidebar.core.sites'`
- `frontend/i18n/locales/fr.json:31` : `"core": { "sites": "Sites" }`

La regle `naming.md` impose `sidebar.<module>.*` pour les cles de sidebar. L'item est declare comme appartenant au module `sites` (`'module' => 'sites'`), la cle i18n devrait donc etre `sidebar.sites.admin` (ou `sidebar.sites.sites` / `sidebar.sites.list`).

**Correction** :

```json
// frontend/i18n/locales/fr.json
"sidebar": {
    "core": {
        "roles": "Gestion des rôles",
        "users": "Utilisateurs",
        "audit_log": "Journal d'audit"
    },
    "sites": {
        "admin": "Sites"
    }
}
```

```php
// config/sidebar.php
[
    'label' => 'sidebar.sites.admin',
    'to'    => '/admin/sites',
    'icon'  => 'mdi:domain',
    'module'=> 'sites',
    'permission' => 'sites.view',
],
```

---

### 3.3 MOYEN — `UserPasswordHasherProcessor` et `MeProvider` non `final`

**Fichiers** :
- `src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserPasswordHasherProcessor.php:16`
- `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/MeProvider.php:14`

Ce sont les deux seules classes ApiPlatform de la PR qui ne sont pas `final`. Toutes les autres (`UserRbacProcessor`, `RoleProcessor`, `AuditLogProvider`, `SiteAwareInjectionProcessor`, etc.) le sont. Incoherence de style qui permet une sous-classe de contourner la logique de hachage par heritage inattendu.

**Correction** : ajouter `final` et passer `readonly` tant qu'on y est.

```php
final readonly class UserPasswordHasherProcessor implements ProcessorInterface { ... }
final readonly class MeProvider implements ProviderInterface { ... }
```

Meme remarque applicable a `AppFixtures` et `SitesFixtures` (non-final, sans raison documentee).

---

### 3.4 MINEUR — Couplage inter-modules (Core → Sites) dans `User`, fixtures, commande seed

**Fichiers** :
- `src/Module/Core/Domain/Entity/User.php:23` — PHPDoc `@var Collection<int, Site>`
- `src/Module/Core/Infrastructure/DataFixtures/AppFixtures.php:12` — import `SiteRepositoryInterface`, `SitesFixtures`
- `src/Module/Core/Infrastructure/Console/SeedE2ECommand.php:12` — import `SiteRepositoryInterface`

La regle #1 (`CLAUDE.md`) interdit l'import direct d'un module vers un autre. Ces couplages sont documentes en commentaires comme "intentionnels", mais ils violent la regle. Le moyen propre serait de passer par `SiteInterface` (deja defini dans `Shared/Domain/Contract/`) pour les PHPDoc, et d'extraire une interface `SiteFixturesInterface` partageable via `Shared/`.

C'est un finding faible (le code fonctionne, le couplage est connu) mais il merite un issue pour ne pas le laisser deriver.

---

## 4. Incoherences de patterns

### 4.1 MOYEN — `debounce` reimplemente localement dans `audit-log.vue`

**Fichier** : `frontend/modules/core/pages/admin/audit-log.vue:306-312`

```typescript
function debounce<T extends (...args: never[]) => void>(fn: T, delay: number): T {
    let timer: ReturnType<typeof setTimeout> | null = null
    return ((...args: Parameters<T>) => {
        if (null !== timer) clearTimeout(timer)
        timer = setTimeout(() => fn(...args), delay)
    }) as T
}
```

Utile et correct, mais vit dans le composant au lieu de `frontend/shared/utils/debounce.ts`. Si une autre page ajoute un debounce, on va dupliquer. Il y a deja `color.ts` dans `shared/utils/` comme exemple de mini-util testee — `debounce.ts` a sa place a cote.

**Correction** : extraire vers `frontend/shared/utils/debounce.ts` avec un test Vitest minimal.

---

### 4.2 MOYEN — `relativeDate` plafonne a la semaine

**Fichier** : `frontend/shared/components/audit/AuditTimeline.vue:171-181`

```typescript
if (absSec < 604800) return fmt.format(..., 'day')
return fmt.format(..., 'week')    // <-- au-dela, tout est en semaines
```

Une entree d'il y a 1 an affichera *"il y a 52 semaines"*. Peu lisible. Il manque les paliers `month` et `year`.

**Correction** :

```typescript
if (absSec < 60)      return fmt.format(..., 'second')
if (absSec < 3600)    return fmt.format(..., 'minute')
if (absSec < 86400)   return fmt.format(..., 'hour')
if (absSec < 604800)  return fmt.format(..., 'day')
if (absSec < 2592000) return fmt.format(..., 'week')   // < 30j
if (absSec < 31536000) return fmt.format(..., 'month') // < 365j
return fmt.format(..., 'year')
```

---

### 4.3 MOYEN — `entityType` affiche brut dans le drawer d'audit

**Fichier** : `frontend/modules/core/pages/admin/audit-log.vue:138-139`

```html
<h3 class="text-sm font-medium text-gray-700 mb-2">
    {{ selectedEntry.entityType }} #{{ selectedEntry.entityId }}
</h3>
```

Affiche `core.User #42`, `sites.Site #7`, etc. La cle i18n `audit.entity.user` existe deja dans `fr.json:79` mais n'est pas utilisee. La spec `doc/audit-log.md` mentionne ce lookup.

**Correction** :

```vue
<script setup lang="ts">
function formatEntityType(type: string): string {
    const key = `audit.entity.${type.toLowerCase().replace('.', '_')}`
    return te(key) ? t(key) : type
}
</script>

<template>
    <h3>{{ formatEntityType(selectedEntry.entityType) }} #{{ selectedEntry.entityId }}</h3>
</template>
```

Et ajouter les cles manquantes dans `fr.json` :

```json
"audit": {
    "entity": {
        "core_user": "Utilisateur",
        "core_role": "Rôle",
        "core_permission": "Permission",
        "sites_site": "Site"
    }
}
```

---

### 4.4 MINEUR — `loadSidebar()` recharge inutile a chaque switch de site

**Fichier** : `frontend/modules/sites/composables/useCurrentSite.ts:94-97`

```typescript
await loadSidebar()  // apres chaque switch
```

Commentaire : *"les filtres de modules peuvent dependre du site courant"*. En pratique, dans `config/sidebar.php` de Starseed aucun item ne depend du site. C'est un aller-retour reseau inutile a chaque switch, et la sidebar peut "flicker" pour l'utilisateur.

**Correction** : rendre le rechargement opt-in ou documenter la raison actuelle (prevoir le futur).

```typescript
// La sidebar ne depend actuellement d'aucun site, mais le /api/sidebar
// pourrait devenir site-scoped dans le futur (ex: items RH par site).
// On garde le reload pour etre defensif — cout : 1 RTT par switch (~100ms).
await loadSidebar()
```

Ou le supprimer et ajouter un commit en passant : le jour ou la sidebar devient site-scoped, on le reintroduira.

---

### 4.5 MINEUR — Alias de retrocompat `SiteNotAuthorizedException` sans planning de suppression

**Fichier** : `src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php`

Classe `final` vide qui etend `App\Shared\Domain\Exception\SiteNotAuthorizedException`. Aucun usage restant dans la branche — c'est une dette technique a supprimer.

**Correction** : rechercher les usages (`grep -r 'Sites\\Domain\\Exception\\SiteNotAuthorizedException'`), les remplacer, puis supprimer le fichier.

---

## 5. Documentation et configuration

### 5.1 MINEUR — `CHANGELOG.md` non mis a jour

**Fichier** : `CHANGELOG.md`

Toujours bloque sur `## [0.0.0]` avec un contenu pre-PR. Aucun resume de la feature audit-log, du module Sites, du systeme RBAC.

**Correction** : ajouter des entrees `## [0.1.34]` (ou la version courante au merge) avec les sections `Added`, `Changed`, `Fixed`.

---

### 5.2 MINEUR — `AuditLogEntityTypesResource` a un `id` hardcode inutile

**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogEntityTypesResource.php:31`

```php
public readonly string $id = 'entity-types';
```

Le provider ne lit pas `$uriVariables['id']`. Ce champ est du bruit dans le DTO. Si quelqu'un regarde la reponse JSON en pensant "tiens, quel est cet id ?", il perd du temps.

**Correction** : supprimer la propriete `$id`.

---

### 5.3 MINEUR — Commentaire incorrect sur l'escape LIKE en PostgreSQL

**Fichier** : `src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php:175-176`

Voir 1.5. Le commentaire affirme une propriete fausse de PostgreSQL. A corriger avec la fix du filtre.

---

## 6. Frontend et UX

### 6.1 MINEUR — Trop de state loading/error pour les drawers, pas d'UX "network-retry"

Les drawers `UserRbacDrawer`, `RoleDrawer`, `SiteDrawer` ont un pattern `loadFailed = true` → reset des listes en cas d'erreur. Bon point pour eviter les donnees stale. Mais aucun bouton "Reessayer" n'est offert : l'utilisateur doit fermer et rouvrir le drawer pour relancer le fetch. Un bouton `MalioButton` "Reessayer" dans l'etat erreur ameliorerait l'UX.

Non bloquant, juste une suggestion pour la prochaine iteration.

---

### 6.2 MINEUR — `onMounted` dans `logout.vue` n'a pas de garde contre la double execution

**Fichier** : `frontend/modules/core/pages/logout.vue:16-32`

Si la page `logout` est visitee deux fois rapidement (click-click ou navigation keep-alive), `auth.logout()` est appele deux fois en parallele. Le backend Lexik JWT logout est idempotent donc c'est inoffensif, mais on peut voir deux toasts d'erreur si le reseau tombe pile entre les deux.

Pas critique. A signaler pour info.

---

## 7. Bonnes pratiques a retenir

### Ce qui est vraiment bien fait dans cette PR

1. **Pattern swap-and-clear dans `AuditListener::postFlush`** — La copie locale de `$pendingLogs` puis le vidage immediat avant l'iteration proteje contre les flushs re-entrants. Le try/catch par entree garantit qu'une erreur d'audit ne casse jamais le flow metier. C'est exactement ce que la spec demandait, implemente correctement.

2. **Connexion DBAL dediee `audit` avec propagation du suffixe `_test`** — Piege classique rate dans beaucoup de projets : la connexion secondaire ecrit dans la base dev pendant que l'ORM ecrit dans la base test. Ici, `doctrine.yaml` propage `dbname_suffix` aux deux connexions en environnement test + `idle_connection_ttl: 1` pour ne pas saturer le pool. Propre.

3. **Trois miroirs RBAC parfaitement synchronises** — `config/sidebar.php` + `frontend/tests/e2e/_fixtures/personas.ts` + `src/Module/Core/Infrastructure/Console/SeedE2ECommand.php`. Les 6 personas et les 4 liens admin (`users`, `roles`, `sites`, `audit-log`) matchent a la ligne pres. C'est la regle la plus dure a tenir sur la duree.

4. **Protection `AdminHeadcountGuard` avec limitation TOCTOU documentee honnetement** — Le commentaire du guard cite explicitement le risque accepte plutot que de le cacher. Pour un CRM interne mono-operateur, c'est la bonne decision d'architecture.

5. **`useAuditLog` s'auto-enregistre via `onAuthSessionCleared`** — Respecte la regle "composable singleton = reset au logout". Idem pour `useSidebar`, `useModules`, `useCurrentSite`. Discipline appliquee partout.

6. **Pagination cappee sur `AuditLogResource`** (`paginationMaximumItemsPerPage: 50`) — Bon reflexe defensif contre les requetes abusives sur le volume appele a croitre.

7. **Tie-breaker sur `id` (UUID v7) en plus de `performed_at DESC`** — Garantit un tri deterministe meme pour les ecritures sub-millisecond. Detail rare qui evite un bug de pagination futur.

8. **`AuditLogResource` est read-only stricte** (aucun POST/PUT/PATCH/DELETE) — Conforme au caractere append-only documente. Le 405 est automatique.

### Les 10 regles a graver (tirees des findings)

1. **Ne jamais laisser `/api/docs` publique en prod** — c'est une carte offerte gratuitement a un attaquant.
2. **Toujours poser les en-tetes de securite de base** (X-Frame-Options, X-Content-Type-Options, Referrer-Policy, HSTS) — 3 lignes de Nginx, impact enorme.
3. **Toujours typer les parametres DBAL** (`Types::DATETIMETZ_IMMUTABLE` et compagnie) — passer une string brute a une colonne typee est un bug en attente.
4. **LIKE/ILIKE avec input utilisateur = toujours clause `ESCAPE` explicite** — ne pas se fier au comportement par defaut.
5. **`instanceof` + comportement "OK si pas du bon type" = faille** — une absence de type doit lever, pas passer.
6. **Tout `await` dans un callback qui modifie un flag singleton = `try/finally`** — sinon un throw bloque le flag.
7. **Toujours poser `paginationMaximumItemsPerPage`** sur les ressources exposees — sinon c'est un DoS en un query param.
8. **`JSON.stringify` sur donnees externes = toujours try/catch** — les objets circulaires existent.
9. **Cles i18n doivent suivre le namespace du module owner** (`sidebar.<module>.*`) — sinon on accumule des cles orphelines.
10. **`final` par defaut sur les services applicatifs** — ouverture a l'heritage = decision explicite, pas oublie.

---

## 8. Resume par priorite

| Priorite | Section | Probleme | Fichier |
|----------|---------|----------|---------|
| **P0** | 1.1 | `/api/docs` accessible public en prod | `config/packages/security.yaml:46` |
| **P0** | 1.2 | Aucun en-tete de securite HTTP en prod | `infra/prod/nginx.conf`, `nginx-proxy.conf` |
| **P0** | 1.3 | `robots.txt` autorise l'indexation | `frontend/public/robots.txt` |
| **P1** | 1.4 | `performed_at` sans typage → crash 500 | `AuditLogProvider.php:182-186` |
| **P1** | 1.5 | ILIKE sans clause `ESCAPE` | `AuditLogProvider.php:177-180` |
| **P1** | 1.6 | `SiteAwareInjectionProcessor` bypass silencieux | `SiteAwareInjectionProcessor.php:71` |
| **P1** | 1.7 | `isHandlingUnauthorized` sans try/finally | `useApi.ts:125-130` |
| **P1** | 1.8 | `itemsPerPage:999` no-op + pas de cap | `UserRbacDrawer.vue:235-236`, `RoleDrawer.vue:149`, `sites.vue:117` |
| **P1** | 2.1 | `JSON.stringify` sans garde | `AuditLogDetail.vue` |
| **P2** | 2.2 | Log manquant si JSON body invalide | `UserRbacProcessor.php:241-248` |
| **P2** | 3.1 | `<button>` brut au lieu de `MalioButton` | `AuditTimeline.vue:80` |
| **P2** | 3.2 | Cle i18n sous mauvais namespace | `sidebar.php:82`, `fr.json:31` |
| **P2** | 3.3 | Classes non `final` incoherentes | `UserPasswordHasherProcessor.php`, `MeProvider.php` |
| **P2** | 4.1 | `debounce` duplique local | `audit-log.vue:306-312` |
| **P2** | 4.2 | `relativeDate` plafonne a la semaine | `AuditTimeline.vue:181` |
| **P2** | 4.3 | `entityType` non traduit | `audit-log.vue:138-139` |
| **P3** | 3.4 | Couplage inter-modules Core→Sites | `User.php:23`, `AppFixtures.php:12`, `SeedE2ECommand.php:12` |
| **P3** | 4.4 | `loadSidebar()` inutile apres switch site | `useCurrentSite.ts:94-97` |
| **P3** | 4.5 | Alias `SiteNotAuthorizedException` | `Sites/Domain/Exception/` |
| **P3** | 5.1 | CHANGELOG non mis a jour | `CHANGELOG.md` |
| **P3** | 5.2 | `id` hardcode dans `AuditLogEntityTypesResource` | ligne 31 |
| **P3** | 6.1 | Pas de bouton "Reessayer" sur drawer erreur | drawers |
| **P3** | 6.2 | Double execution `onMounted` logout | `logout.vue:16-32` |

**3 P0** (securite prod), **6 P1** (bugs silencieux + impact utilisateur), **6 P2** (qualite/conventions), **7 P3** (polish/dette). Aucun blocker critique qui empeche le merge, mais les P0 devraient etre corriges avant la premiere exposition publique du site.

---

> Voir `TICKETS.md` pour les tickets actionnables.
