feat: gate the upload replacement file behind a feature flag

.env

@@ -114,8 +114,7 @@ FEATURE_SANITIZE_UPLOADS=false
 FEATURE_STAMP_INVOICES=false
 FEATURE_DATEV_EMAIL=false
 FEATURE_MYEP_LOGIN=false
-
-UPLOAD_REPLACEMENT_FILE=assets/pdf/chicken.pdf
+FEATURE_UPLOAD_REPLACEMENT=false
 
 MAINTENANCE_MODE_ENABLED=false
 MAINTENANCE_MODE_IP_WHITELIST=188.245.62.21

config/packages/flagception.yaml

@@ -13,3 +13,6 @@ flagception:
         myep_login:
             default: false
             env: FEATURE_MYEP_LOGIN
+        upload_replacement:
+            default: false
+            env: FEATURE_UPLOAD_REPLACEMENT

config/services.yaml

@@ -50,6 +50,11 @@ parameters:
 
     teamer_inactive_period: '-2 years'
 
+    # The stand-in PDF served instead of real uploads while the upload_replacement feature is
+    # on. Relative to the project dir. Which file is a fixed fact of the repo; whether it is
+    # used at all is the feature flag's job alone.
+    upload_replacement_file: 'assets/pdf/chicken.pdf'
+
     # The contract upload period, counted from the creation of the disposition. This is the
     # single definition: the reminder mail, the "Honorarvertrag fällig" flag on the teamer
     # dashboard, the admin list of overdue contracts and the wording of the mails all read it,
@@ -262,8 +267,7 @@ services:
         arguments:
             $orphanageManager: '@oneup_uploader.orphanage_manager'
             $projectDir: '%kernel.project_dir%'
-            $environment: '%kernel.environment%'
-            $uploadReplacementFile: '%env(default::UPLOAD_REPLACEMENT_FILE)%'
+            $uploadReplacementFile: '%upload_replacement_file%'
 
     App\Service\Pdf\Sanitizer:
         arguments:

src/Service/Upload/UploadHandler.php

@@ -6,6 +6,7 @@ use App\Entity\Upload;
 use App\Model\UploadDto;
 use App\Model\UploadSessionDto;
 use App\Service\Pdf\Sanitizer;
+use Flagception\Manager\FeatureManagerInterface;
 use Oneup\UploaderBundle\Uploader\Orphanage\OrphanageManager;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Filesystem\Exception\IOException;
@@ -21,12 +22,9 @@ class UploadHandler
         private readonly OrphanageManager $orphanageManager,
         private readonly Sanitizer $sanitizer,
         private readonly LoggerInterface $logger,
+        private readonly FeatureManagerInterface $featureManager,
         private readonly string $projectDir,
-        private readonly string $environment,
-        // Bound from %env(default::UPLOAD_REPLACEMENT_FILE)%, whose empty fallback parameter
-        // resolves to null when the variable is not set at all - so "unset", "set to empty" and
-        // "absent" all have to mean the same thing here.
-        private readonly ?string $uploadReplacementFile,
+        private readonly string $uploadReplacementFile,
     ) {
     }
 
@@ -166,23 +164,22 @@ class UploadHandler
     }
 
     /**
-     * Returns the upload file path, substituting a replacement file in non-production environments
-     * when configured. Use this for read-only access (downloads, email attachments).
+     * Returns the upload file path, substituting the stand-in document while the
+     * upload_replacement feature is on. Use this for read-only access (downloads, email
+     * attachments); getUploadFilepath() stays the real location and is what writes target.
      */
     public function getResolvedUploadFilepath(Upload $upload): string
     {
-        $originalPath = $this->getUploadFilepath($upload);
-
-        if (null === $this->uploadReplacementFile
-            || '' === $this->uploadReplacementFile
-            || 'prod' === $this->environment) {
-            return $originalPath;
+        if (false === $this->featureManager->isActive('upload_replacement')) {
+            return $this->getUploadFilepath($upload);
         }
 
         $replacementPath = sprintf('%s/%s', $this->projectDir, $this->uploadReplacementFile);
 
         if (false === file_exists($replacementPath)) {
-            return $originalPath;
+            // Falling back to the real document here is what made the old behaviour opaque: the
+            // feature would look active and quietly serve the genuine file instead.
+            throw new \RuntimeException('Upload replacement file not found at '.$replacementPath);
         }
 
         return $replacementPath;

tests/Service/Upload/UploadHandlerTest.php

@@ -0,0 +1,105 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Service\Upload;
+
+use App\Entity\Upload;
+use App\Service\Pdf\Sanitizer;
+use App\Service\Upload\UploadHandler;
+use Flagception\Manager\FeatureManagerInterface;
+use Oneup\UploaderBundle\Uploader\Orphanage\OrphanageManager;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+/**
+ * The stand-in document is gated by the upload_replacement feature and by nothing else - not by
+ * the environment, not by whether a path happens to be configured. That, and the fact that a
+ * missing stand-in is an error rather than a silent fall back to the genuine file, is what is
+ * pinned here.
+ */
+class UploadHandlerTest extends TestCase
+{
+    private const PROJECT_DIR = '/srv/app';
+    private const REPLACEMENT_FILE = 'assets/pdf/chicken.pdf';
+
+    private FeatureManagerInterface&MockObject $featureManager;
+
+    protected function setUp(): void
+    {
+        $this->featureManager = $this->createMock(FeatureManagerInterface::class);
+    }
+
+    public function testTheRealDocumentIsServedWhileTheFeatureIsOff(): void
+    {
+        $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(false);
+
+        $this->assertSame(
+            '/srv/app/uploads/invoice/i/invoice-1.pdf',
+            $this->handler()->getResolvedUploadFilepath($this->upload())
+        );
+    }
+
+    public function testTheStandInIsServedWhileTheFeatureIsOn(): void
+    {
+        $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(true);
+
+        // the real file of the repository, so the path resolves to something that exists
+        $handler = $this->handler(\dirname(__DIR__, 3), self::REPLACEMENT_FILE);
+
+        $this->assertSame(
+            \dirname(__DIR__, 3).'/'.self::REPLACEMENT_FILE,
+            $handler->getResolvedUploadFilepath($this->upload())
+        );
+    }
+
+    public function testAMissingStandInFailsInsteadOfServingTheRealDocument(): void
+    {
+        $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(true);
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('/srv/app/assets/pdf/nope.pdf');
+
+        $this->handler(self::PROJECT_DIR, 'assets/pdf/nope.pdf')
+            ->getResolvedUploadFilepath($this->upload())
+        ;
+    }
+
+    /**
+     * The real location is never substituted: it is what writes target.
+     */
+    public function testTheUploadFilepathIgnoresTheFeature(): void
+    {
+        $this->featureManager->method('isActive')->with('upload_replacement')->willReturn(true);
+
+        $this->assertSame(
+            '/srv/app/uploads/invoice/i/invoice-1.pdf',
+            $this->handler()->getUploadFilepath($this->upload())
+        );
+    }
+
+    private function handler(
+        string $projectDir = self::PROJECT_DIR,
+        string $replacementFile = self::REPLACEMENT_FILE,
+    ): UploadHandler {
+        return new UploadHandler(
+            $this->createMock(RequestStack::class),
+            $this->createMock(OrphanageManager::class),
+            $this->createMock(Sanitizer::class),
+            new NullLogger(),
+            $this->featureManager,
+            $projectDir,
+            $replacementFile,
+        );
+    }
+
+    private function upload(): Upload
+    {
+        return (new Upload())
+            ->setType(Upload::TYPE_INVOICE)
+            ->setFilename('invoice-1.pdf')
+        ;
+    }
+}