From a579ee63783f733be3d06c758558aeded8cc5f9f Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 15 Jun 2026 16:56:15 +0200 Subject: [PATCH] fix(shared) : durcissement infra upload (ERP-154) - supprime le fichier orphelin sur disque si la persistance Doctrine echoue apres l ecriture (FileUploader::remove + compensation dans le processor) - gere hash_file() renvoyant false (fichier illisible) -> FileUploadException au lieu d un TypeError opaque - documente le choix de securite du GET (infra generique, cloisonnement delegue au consommateur) --- src/Shared/Domain/Entity/UploadedDocument.php | 8 ++++++- .../State/UploadedDocumentProcessor.php | 14 ++++++++++++- .../Infrastructure/Upload/FileUploader.php | 21 +++++++++++++++++++ .../Upload/FileUploaderTest.php | 17 +++++++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/Shared/Domain/Entity/UploadedDocument.php b/src/Shared/Domain/Entity/UploadedDocument.php index f393127..e1151cd 100644 --- a/src/Shared/Domain/Entity/UploadedDocument.php +++ b/src/Shared/Domain/Entity/UploadedDocument.php @@ -33,7 +33,13 @@ use Symfony\Component\Serializer\Attribute\Groups; * bornage taille, checksum, ecriture disque) puis persiste. MIME hors * whitelist -> 422. * - Get (/uploaded_documents/{id}) : necessaire pour qu'API Platform genere - * l'IRI renvoyee par le Post. Securisee par l'authentification globale /api. + * l'IRI renvoyee par le Post. Protege par IS_AUTHENTICATED_FULLY uniquement + * (pas de RBAC ni de cloisonnement tenant ici) : cette ressource est une + * infra GENERIQUE qui ne porte aucune notion de proprietaire metier. Le + * cloisonnement d'acces (qui peut voir quel document) est volontairement + * delegue au module CONSOMMATEUR (ex: la Decharge M4), qui exposera le + * document via sa propre ressource cloisonnee plutot que via cet endpoint + * technique. Ne renvoie que des metadonnees (jamais le binaire). * * Pas de GetCollection exposee (non requise) — la regle de pagination ne * s'applique donc pas ici. diff --git a/src/Shared/Infrastructure/ApiPlatform/State/UploadedDocumentProcessor.php b/src/Shared/Infrastructure/ApiPlatform/State/UploadedDocumentProcessor.php index b4a5398..26e9d2b 100644 --- a/src/Shared/Infrastructure/ApiPlatform/State/UploadedDocumentProcessor.php +++ b/src/Shared/Infrastructure/ApiPlatform/State/UploadedDocumentProcessor.php @@ -29,6 +29,9 @@ use Symfony\Component\Security\Core\User\UserInterface; * - fichier absent -> 422 ; * - MIME hors whitelist / fichier trop volumineux (FileUploadException) -> 422. * + * Si la persistance Doctrine echoue APRES l'ecriture disque, le fichier physique + * deja deplace est supprime (compensation) pour ne pas laisser de binaire orphelin. + * * @implements ProcessorInterface */ final class UploadedDocumentProcessor implements ProcessorInterface @@ -63,6 +66,15 @@ final class UploadedDocumentProcessor implements ProcessorInterface $document->setCreatedBy($user); } - return $this->persistProcessor->process($document, $operation, $uriVariables, $context); + try { + return $this->persistProcessor->process($document, $operation, $uriVariables, $context); + } catch (\Throwable $e) { + // La persistance a echoue APRES l'ecriture disque (erreur DB, FK...) : + // on supprime le fichier orphelin pour ne pas le laisser sans ligne + // uploaded_document correspondante, puis on relaie l'erreur. + $this->fileUploader->remove($document); + + throw $e; + } } } diff --git a/src/Shared/Infrastructure/Upload/FileUploader.php b/src/Shared/Infrastructure/Upload/FileUploader.php index eef52d7..a7f61f5 100644 --- a/src/Shared/Infrastructure/Upload/FileUploader.php +++ b/src/Shared/Infrastructure/Upload/FileUploader.php @@ -6,6 +6,7 @@ namespace App\Shared\Infrastructure\Upload; use App\Shared\Domain\Entity\UploadedDocument; use App\Shared\Domain\Exception\FileTooLargeException; +use App\Shared\Domain\Exception\FileUploadException; use App\Shared\Domain\Exception\UnsupportedMimeTypeException; use Symfony\Component\Clock\ClockInterface; use Symfony\Component\DependencyInjection\Attribute\Autowire; @@ -78,7 +79,13 @@ final class FileUploader } // Checksum AVANT le move : le chemin du fichier change apres deplacement. + // hash_file renvoie false si le fichier temporaire est illisible (I/O) : + // on echoue proprement plutot que de propager un TypeError opaque au + // constructeur (parametre $checksum type string). $checksum = hash_file('sha256', $file->getPathname()); + if (false === $checksum) { + throw new FileUploadException('Impossible de lire le fichier televerse pour en calculer l\'empreinte.'); + } $now = $this->clock->now(); $relativeDir = $now->format('Y').'/'.$now->format('m'); @@ -104,4 +111,18 @@ final class FileUploader createdAt: $now, ); } + + /** + * Supprime le fichier physique d'un document (compensation lorsque la + * persistance echoue APRES l'ecriture disque). Best-effort : silencieux si + * le fichier a deja disparu. Evite d'accumuler des binaires orphelins non + * references en base. + */ + public function remove(UploadedDocument $document): void + { + $path = $this->uploadBaseDir.'/'.$document->getStoredPath(); + if (is_file($path)) { + @unlink($path); + } + } } diff --git a/tests/Shared/Infrastructure/Upload/FileUploaderTest.php b/tests/Shared/Infrastructure/Upload/FileUploaderTest.php index 8f78f5a..90c8f6c 100644 --- a/tests/Shared/Infrastructure/Upload/FileUploaderTest.php +++ b/tests/Shared/Infrastructure/Upload/FileUploaderTest.php @@ -93,6 +93,23 @@ final class FileUploaderTest extends TestCase ); } + public function testRemoveDeletesStoredFile(): void + { + $uploader = $this->createUploader(); + $file = $this->makeUploadedFile($this->minimalPdf(), 'facture.pdf'); + $document = $uploader->upload($file); + $storedPath = $this->uploadBaseDir.'/'.$document->getStoredPath(); + self::assertFileExists($storedPath); + + // Compensation : remove() efface le fichier physique... + $uploader->remove($document); + self::assertFileDoesNotExist($storedPath); + + // ...et reste silencieux si on le rappelle alors que le fichier a disparu. + $uploader->remove($document); + self::assertFileDoesNotExist($storedPath); + } + private function createUploader(?MockClock $clock = null): FileUploader { return new FileUploader($this->uploadBaseDir, $clock ?? new MockClock());