fix(shared) : durcissement infra upload (ERP-154)
Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Failing after 50s
Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m25s

- 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)
This commit is contained in:
Matthieu
2026-06-15 16:56:15 +02:00
parent b989c33cc4
commit a579ee6378
4 changed files with 58 additions and 2 deletions
@@ -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.
@@ -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<mixed, mixed>
*/
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;
}
}
}
@@ -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);
}
}
}
@@ -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());