From 2ac815d074faba508019273a4fc6705aabf536eb Mon Sep 17 00:00:00 2001
From: Matthieu <mtholot19@gmail.com>
Date: Tue, 17 Mar 2026 15:23:18 +0100
Subject: [PATCH] fix(security) : block SVG upload, enforce ROLE_CLIENT
 restrictions on documents

- Block SVG MIME type in TaskDocumentProcessor upload validation
- Serve existing SVG files as attachment (defense-in-depth) in download controller
- Block ROLE_CLIENT from uploading documents to tasks (only allowed via portal tickets)
- Add Doctrine extension to filter projects by allowedProjects for ROLE_CLIENT

Tickets: T-003, T-005, T-006

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 .../TaskDocumentDownloadController.php        |  9 ++-
 src/Doctrine/ProjectAllowedExtension.php      | 66 +++++++++++++++++++
 src/State/TaskDocumentProcessor.php           |  9 ++-
 3 files changed, 79 insertions(+), 5 deletions(-)
 create mode 100644 src/Doctrine/ProjectAllowedExtension.php

diff --git a/src/Controller/TaskDocumentDownloadController.php b/src/Controller/TaskDocumentDownloadController.php
index 7ec344c..a5ecd23 100644
--- a/src/Controller/TaskDocumentDownloadController.php
+++ b/src/Controller/TaskDocumentDownloadController.php
@@ -51,9 +51,12 @@ class TaskDocumentDownloadController extends AbstractController
         $mimeType = $document->getMimeType() ?? 'application/octet-stream';
 
         // Inline for images and PDFs, attachment for everything else
-        $disposition = str_starts_with($mimeType, 'image/') || 'application/pdf' === $mimeType
-            ? ResponseHeaderBag::DISPOSITION_INLINE
-            : ResponseHeaderBag::DISPOSITION_ATTACHMENT;
+        // SVG files are always served as attachment to prevent XSS via embedded JavaScript
+        $disposition = 'image/svg+xml' === $mimeType
+            ? ResponseHeaderBag::DISPOSITION_ATTACHMENT
+            : (str_starts_with($mimeType, 'image/') || 'application/pdf' === $mimeType
+                ? ResponseHeaderBag::DISPOSITION_INLINE
+                : ResponseHeaderBag::DISPOSITION_ATTACHMENT);
 
         $response->setContentDisposition($disposition, $document->getOriginalName());
         $response->headers->set('Content-Type', $mimeType);
diff --git a/src/Doctrine/ProjectAllowedExtension.php b/src/Doctrine/ProjectAllowedExtension.php
new file mode 100644
index 0000000..710bacf
--- /dev/null
+++ b/src/Doctrine/ProjectAllowedExtension.php
@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Doctrine;
+
+use ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface;
+use ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface;
+use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface;
+use ApiPlatform\Metadata\Operation;
+use App\Entity\Project;
+use App\Entity\User;
+use Doctrine\ORM\QueryBuilder;
+use Symfony\Bundle\SecurityBundle\Security;
+
+final readonly class ProjectAllowedExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface
+{
+    public function __construct(
+        private Security $security,
+    ) {}
+
+    public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, ?Operation $operation = null, array $context = []): void
+    {
+        $this->addWhere($queryBuilder, $resourceClass);
+    }
+
+    public function applyToItem(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, array $identifiers, ?Operation $operation = null, array $context = []): void
+    {
+        $this->addWhere($queryBuilder, $resourceClass);
+    }
+
+    private function addWhere(QueryBuilder $queryBuilder, string $resourceClass): void
+    {
+        if (Project::class !== $resourceClass) {
+            return;
+        }
+
+        $user = $this->security->getUser();
+
+        if (!$user instanceof User) {
+            return;
+        }
+
+        // Only restrict for ROLE_CLIENT users who are NOT admins
+        if (!in_array('ROLE_CLIENT', $user->getRoles(), true) || in_array('ROLE_ADMIN', $user->getRoles(), true)) {
+            return;
+        }
+
+        $rootAlias = $queryBuilder->getRootAliases()[0];
+
+        $allowedProjectIds = $user->getAllowedProjects()->map(
+            fn (Project $project) => $project->getId(),
+        )->toArray();
+
+        if ([] === $allowedProjectIds) {
+            $queryBuilder->andWhere('1 = 0');
+
+            return;
+        }
+
+        $queryBuilder
+            ->andWhere($rootAlias.'.id IN (:allowed_project_ids)')
+            ->setParameter('allowed_project_ids', $allowedProjectIds)
+        ;
+    }
+}
diff --git a/src/State/TaskDocumentProcessor.php b/src/State/TaskDocumentProcessor.php
index d32d3cb..7359fb8 100644
--- a/src/State/TaskDocumentProcessor.php
+++ b/src/State/TaskDocumentProcessor.php
@@ -25,7 +25,7 @@ final readonly class TaskDocumentProcessor implements ProcessorInterface
     private const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50 MB
 
     private const ALLOWED_MIME_TYPES = [
-        'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/svg+xml',
+        'image/jpeg', 'image/png', 'image/gif', 'image/webp',
         'application/pdf',
         'application/msword',
         'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
@@ -40,7 +40,7 @@ final readonly class TaskDocumentProcessor implements ProcessorInterface
 
     private const MIME_TO_EXTENSION = [
         'image/jpeg'                                                                => 'jpg', 'image/png' => 'png', 'image/gif' => 'gif',
-        'image/webp'                                                                => 'webp', 'image/svg+xml' => 'svg',
+        'image/webp'                                                                => 'webp',
         'application/pdf'                                                           => 'pdf',
         'application/msword'                                                        => 'doc',
         'application/vnd.openxmlformats-officedocument.wordprocessingml.document'   => 'docx',
@@ -92,6 +92,11 @@ final readonly class TaskDocumentProcessor implements ProcessorInterface
         $clientTicket = null;
 
         if ('' !== $taskIri) {
+            // ROLE_CLIENT (without ROLE_ADMIN) cannot upload documents directly to tasks
+            if ($this->security->isGranted('ROLE_CLIENT') && !$this->security->isGranted('ROLE_ADMIN')) {
+                throw new AccessDeniedHttpException('Clients can only upload documents to client tickets.');
+            }
+
             $task = $this->entityManager->getRepository(Task::class)->find((int) basename($taskIri));
 
             if (null === $task) {