fix(audit-log) : reset pendingLogs sur onFlush + valide filtres + documente contrat rollback

Trois corrections issues du code review multi-agent sur la PR audit-log :

- AuditListener : reset defensif de pendingLogs en debut de onFlush. Si
  un flush precedent a leve une exception avant postFlush (qui n'est
  jamais appele sur un flush rate), le state listener gardait des
  changements jamais committes, ecrits a tort par le prochain postFlush
  reussi — audit_log pouvait donc contenir des lignes decrivant des
  evenements qui n'ont pas eu lieu en DB. Test de regression via
  Reflection pour injecter un log orphelin et verifier qu'il n'arrive
  pas dans audit_log.

- AuditLogProvider : validation explicite des filtres performed_at[after]
  et performed_at[before] (strtotime) + whitelist stricte sur `action`
  (create|update|delete). Avant, un input malforme remontait jusqu'a
  Postgres et faisait un 500 (SQLSTATE[22007]). Desormais 400 explicite,
  pas de log pollue.

- doc/audit-log.md : ajoute une section "Contrat" qui explicite ce que
  audit_log garantit (journal des intentions appliquees par l'ORM) et ne
  garantit PAS (reflet exact du commit outermost — une ligne audit peut
  persister si une transaction outermost rollback apres un flush inner
  reussi, parce que l'audit ecrit sur une connexion DBAL dediee).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-04-22 19:34:22 +02:00
parent d3e30e55b2
commit e0624eace0
5 changed files with 127 additions and 6 deletions

View File

@@ -188,6 +188,27 @@ final class AuditLogApiTest extends AbstractApiTestCase
self::assertSame($row['id'], $data['id']);
}
/**
* Validation des filtres : un input malforme doit retourner un 400
* explicite, pas un 500 (Postgres qui rejette le cast timestamp) ni
* un match silencieux sur une valeur inattendue.
*/
public function testInvalidPerformedAtFilterReturns400(): void
{
$client = $this->authenticatedClient('admin', 'admin');
$response = $client->request('GET', '/api/audit-logs?performed_at[after]=pas-une-date');
self::assertSame(400, $response->getStatusCode());
}
public function testInvalidActionFilterReturns400(): void
{
$client = $this->authenticatedClient('admin', 'admin');
$response = $client->request('GET', '/api/audit-logs?action=dropTable');
self::assertSame(400, $response->getStatusCode());
}
public function testPostIsNotAllowed(): void
{
$client = $this->authenticatedClient('admin', 'admin');

View File

@@ -7,8 +7,11 @@ namespace App\Tests\Module\Core\Infrastructure\Doctrine;
use App\Module\Core\Domain\Entity\Permission;
use App\Module\Core\Domain\Entity\Role;
use App\Module\Core\Domain\Entity\User;
use App\Module\Core\Infrastructure\Doctrine\AuditListener;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use ReflectionProperty;
use stdClass;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
@@ -328,6 +331,59 @@ final class AuditListenerTest extends KernelTestCase
);
}
/**
* Regression : quand un flush precedent a leve une exception avant
* d'atteindre postFlush, `$pendingLogs` reste rempli avec des changements
* jamais committes. Le flush suivant doit les ecraser, pas les fusionner —
* sinon audit_log contient des lignes pour des evenements qui n'ont pas
* eu lieu en base.
*
* Reproduction : on injecte manuellement une entree orpheline dans le
* listener (comme si un flush precedent l'avait capturee puis avait plante),
* on declenche un flush valide, et on verifie que l'orpheline n'apparait
* jamais dans audit_log.
*/
public function testOnFlushResetsStalePendingLogsFromFailedPreviousFlush(): void
{
/** @var AuditListener $listener */
$listener = self::getContainer()->get(AuditListener::class);
// Injecte une entree orpheline : comme si onFlush avait capture ce
// changement, puis que le flush avait plante avant postFlush.
$reflection = new ReflectionProperty($listener, 'pendingLogs');
$reflection->setValue($listener, [[
'entity' => new stdClass(),
'metadata' => null,
'entityType' => 'test.StaleEntity',
'action' => 'create',
'changes' => ['fake' => ['old' => null, 'new' => 'stale']],
'capturedId' => 'stale-id-'.$this->testRunTag,
]]);
// Flush valide qui DOIT re-initialiser pendingLogs avant de capturer
// ses propres changements.
$user = $this->makeUser();
$this->em->persist($user);
$this->em->flush();
$this->createdUserIds[] = $user->getId();
$staleRows = $this->auditConnection->fetchAllAssociative(
'SELECT * FROM audit_log WHERE entity_type = :t',
['t' => 'test.StaleEntity'],
);
self::assertCount(
0,
$staleRows,
'Une entree orpheline d\'un flush precedent ne doit pas fuiter dans audit_log.',
);
// Sanity : le vrai flush a quand meme bien ecrit sa propre ligne.
$userRows = $this->fetchAuditRows($user->getId());
self::assertCount(1, $userRows);
self::assertSame('create', $userRows[0]['action']);
}
/**
* @return list<array{id: string, entity_type: string, entity_id: string, action: string, changes: string}>
*/