Coltura/REVIEW.md

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
2. Bugs silencieux
3. Violations des regles projet
4. Incoherences de patterns
5. Documentation et configuration
6. Frontend et UX
7. Bonnes pratiques a retenir
8. Resume par priorite

---

1. Securite

1.1 CRITIQUE — /api/docs public en production

Fichier : config/packages/security.yaml:46

- { 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 coltura.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.

# 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 :

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) :

server {
    listen 80;
    listen [::]:80;
    server_name coltura.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 :

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 (coltura.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

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 :

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

$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 :

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.

$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.

$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

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.

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

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.

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

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) :

// 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 :

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.

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.

$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

<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.

<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 :

<!-- 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 :

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

// 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.

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

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

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 :

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

<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 :

<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 :

"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

await loadSidebar()  // apres chaque switch

Commentaire : "les filtres de modules peuvent dependre du site courant". En pratique, dans config/sidebar.php de Coltura 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).

// 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

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.