[BUGFIX] Quarantine the domain the request was blocked on

migrations/Version20260804120000.php

@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use App\Utility\RepositoryUrlUtility;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Quarantined entries recorded the host of the clone url, while the check that
+ * blocked them keys on the host of the composer.json url. Recompute the domain
+ * of the existing rows from the push event they carry, so approving them
+ * allowlists the host that is actually consulted.
+ */
+final class Version20260804120000 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Set the quarantined domain to the host of the composer.json url';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $rows = $this->connection->fetchAllAssociative('SELECT id, domain, serialized_push_event FROM documentation_quarantine');
+        foreach ($rows as $row) {
+            try {
+                $pushEvent = json_decode((string) $row['serialized_push_event'], true, 512, JSON_THROW_ON_ERROR);
+            } catch (\JsonException) {
+                continue;
+            }
+            $urlToComposerFile = (string) ($pushEvent['urlToComposerFile'] ?? '');
+            if ('' === $urlToComposerFile) {
+                continue;
+            }
+            $domain = RepositoryUrlUtility::getNormalizedDomain($urlToComposerFile);
+            if ('' === $domain || $domain === $row['domain']) {
+                continue;
+            }
+            $this->connection->update('documentation_quarantine', ['domain' => $domain], ['id' => $row['id']]);
+        }
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->throwIrreversibleMigrationException('The previously stored domain can not be restored, it was derived from the clone url.');
+    }
+
+    public function isTransactional(): bool
+    {
+        return false;
+    }
+}

src/Controller/AdminInterface/Docs/KnownRepositoryDomainsController.php

@@ -13,6 +13,7 @@
 
 use App\Entity\KnownRepositoryDomain;
 use App\Enum\DocumentationRenderingTrigger;
+use App\Exception\DocumentationRenderingRequestDeclinedException;
 use App\Form\KnownDomainCreateType;
 use App\Form\KnownDomainDeleteType;
 use App\Repository\KnownRepositoryDomainRepository;
@@ -69,7 +70,13 @@ public function new(Request $request): Response
             if ($data->isAllowed()) {
                 foreach ($this->documentationQuarantineService->findAllByDomain($data->getDomain()) as $documentationQuarantine) {
                     $pushEvent = $documentationQuarantine->getPushEvent();
-                    $this->renderDocumentationService->requestDocumentationRendering($pushEvent, DocumentationRenderingTrigger::WEB);
+                    try {
+                        $this->renderDocumentationService->requestDocumentationRendering($pushEvent, DocumentationRenderingTrigger::WEB);
+                    } catch (DocumentationRenderingRequestDeclinedException) {
+                        // An entry can be undeployable for reasons that have nothing to
+                        // do with the domain, an irrelevant branch name for instance.
+                        // Keep going, one such entry must not stop the others.
+                    }
 
                     $this->entityManager->remove($documentationQuarantine);
                 }

src/Controller/AdminInterface/Docs/QuarantinedDocumentationsController.php

@@ -71,20 +71,26 @@ public function allow(Request $request, DocumentationQuarantine $quarantinedDocu
             $this->entityManager->persist($knownRepositoryDomain);
             $this->entityManager->flush();
 
+            $declined = 0;
             foreach ($this->documentationQuarantineService->findAllByDomain($domain) as $documentationQuarantine) {
                 $pushEvent = $documentationQuarantine->getPushEvent();
                 try {
                     $this->renderDocumentationService->requestDocumentationRendering($pushEvent, DocumentationRenderingTrigger::WEB);
                 } catch (DocumentationRenderingRequestDeclinedException) {
-                    // Exception is thrown if the request documentation rendering does not comply with requirements
-                    // Intended fall-thru
+                    // An entry can be undeployable for reasons that have nothing to do
+                    // with the domain, an irrelevant branch name for instance. Keep
+                    // going, one such entry must not stop the others.
+                    ++$declined;
                 }
 
                 $this->entityManager->remove($documentationQuarantine);
             }
             $this->entityManager->flush();
 
             $this->addFlash('success', sprintf('The domain %s has been allowed and all quarantined renderings have been activated.', $domain));
+            if ($declined > 0) {
+                $this->addFlash('warning', sprintf('%d of them could not be rendered and were discarded, see the rendering history for the reason.', $declined));
+            }
 
             return $this->redirectToRoute('admin_docs_quarantine_index');
         }

src/Service/DocumentationQuarantineService.php

@@ -14,7 +14,6 @@
 use App\Entity\DocumentationQuarantine;
 use App\Extractor\PushEvent;
 use App\Repository\DocumentationQuarantineRepository;
-use App\Utility\RepositoryUrlUtility;
 use Doctrine\ORM\EntityManagerInterface;
 
 class DocumentationQuarantineService
@@ -32,10 +31,15 @@ public function isQuarantined(PushEvent $pushEvent): bool
         ]);
     }
 
-    public function quarantine(PushEvent $pushEvent): DocumentationQuarantine
+    /**
+     * The domain has to be the one the request was actually blocked on, which is
+     * the host of the composer.json url. It differs from the host of the clone url
+     * for Github always, and can differ for the other services as well.
+     */
+    public function quarantine(PushEvent $pushEvent, string $blockedDomain): DocumentationQuarantine
     {
         $documentationQuarantine = (new DocumentationQuarantine())
-            ->setDomain(RepositoryUrlUtility::getNormalizedDomain($pushEvent->getRepositoryUrl()))
+            ->setDomain($blockedDomain)
             ->setSerializedPushEvent(json_encode($pushEvent, JSON_THROW_ON_ERROR))
             ->setChecksum($this->hash($pushEvent));
 

src/Service/RenderDocumentationService.php

@@ -55,12 +55,12 @@ public function requestDocumentationRendering(PushEvent $pushEvent, Documentatio
         $userIdentifier = $this->security->getUser() instanceof KeyCloakUser ? $this->security->getUser()->getDisplayName() : 'Anon.';
 
         try {
-            $this->documentationBuildInformationService->updateLastHit(RepositoryUrlUtility::getNormalizedDomain($pushEvent->getRepositoryUrl()));
+            $this->documentationBuildInformationService->updateLastHit(RepositoryUrlUtility::getNormalizedDomain($pushEvent->getUrlToComposerFile()));
 
             $composerJson = $this->documentationBuildInformationService->fetchRemoteComposerJson($pushEvent->getUrlToComposerFile());
         } catch (UnknownComposerJsonUrlException $e) {
             if (!$this->documentationQuarantineService->isQuarantined($pushEvent)) {
-                $documentationQuarantine = $this->documentationQuarantineService->quarantine($pushEvent);
+                $documentationQuarantine = $this->documentationQuarantineService->quarantine($pushEvent, $e->normalizedHost);
                 $this->documentationBuildInformationService->notifyAboutUnknownRepositoryDomain($documentationQuarantine);
 
                 $this->historyService->writeHistory(new HistoryEntryDto(

tests/Unit/Service/DocumentationQuarantineServiceTest.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the package t3g/intercept.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace App\Tests\Unit\Service;
+
+use App\Entity\DocumentationQuarantine;
+use App\Extractor\PushEvent;
+use App\Repository\DocumentationQuarantineRepository;
+use App\Service\DocumentationQuarantineService;
+use Doctrine\ORM\EntityManagerInterface;
+use PHPUnit\Framework\TestCase;
+
+class DocumentationQuarantineServiceTest extends TestCase
+{
+    /**
+     * Approving a quarantined entry allowlists the domain it recorded, and that
+     * decision is only correct if the recorded domain is the one the request was
+     * blocked on. For Github those two never match: the clone url is on
+     * github.com while the composer.json is fetched from
+     * raw.githubusercontent.com.
+     */
+    public function testQuarantineRecordsTheDomainTheRequestWasBlockedOn(): void
+    {
+        $persisted = null;
+        $entityManager = $this->createMock(EntityManagerInterface::class);
+        $entityManager->method('persist')->willReturnCallback(
+            static function (object $entity) use (&$persisted): void {
+                $persisted = $entity;
+            }
+        );
+
+        $subject = new DocumentationQuarantineService($entityManager, $this->createMock(DocumentationQuarantineRepository::class));
+        $pushEvent = new PushEvent(
+            'https://github.com/acme/coolextension.git',
+            'main',
+            'https://raw.githubusercontent.com/acme/coolextension/main/composer.json',
+            '{}'
+        );
+
+        $subject->quarantine($pushEvent, 'raw.githubusercontent.com');
+
+        $this->assertInstanceOf(DocumentationQuarantine::class, $persisted);
+        $this->assertSame('raw.githubusercontent.com', $persisted->getDomain());
+    }
+}

tests/Unit/Service/RenderDocumentationServiceTest.php

@@ -0,0 +1,91 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the package t3g/intercept.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace App\Tests\Unit\Service;
+
+use App\Entity\DocumentationQuarantine;
+use App\Enum\DocumentationRenderingTrigger;
+use App\Exception\DocumentationRenderingRequestDeclinedException;
+use App\Exception\UnknownComposerJsonUrlException;
+use App\Extractor\PushEvent;
+use App\Repository\RepositoryBlacklistEntryRepository;
+use App\Service\DocumentationBuildInformationService;
+use App\Service\DocumentationQuarantineService;
+use App\Service\GithubService;
+use App\Service\HistoryService;
+use App\Service\MailService;
+use App\Service\RenderDocumentationService;
+use Doctrine\ORM\EntityManagerInterface;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Symfony\Bundle\SecurityBundle\Security;
+
+class RenderDocumentationServiceTest extends TestCase
+{
+    /**
+     * The quarantined domain is what an admin later allowlists, so it has to be
+     * the host the request was blocked on. That is the host of the composer.json
+     * url, which for Github is never the host of the clone url.
+     */
+    public function testTheBlockedDomainIsHandedToTheQuarantine(): void
+    {
+        $pushEvent = new PushEvent(
+            'https://github.com/acme/coolextension.git',
+            'main',
+            'https://raw.githubusercontent.com/acme/coolextension/main/composer.json',
+            '{}'
+        );
+
+        $buildInformationService = $this->createMock(DocumentationBuildInformationService::class);
+        $buildInformationService->method('fetchRemoteComposerJson')->willThrowException(
+            new UnknownComposerJsonUrlException('', 1782290340, null, $pushEvent->getUrlToComposerFile(), 'raw.githubusercontent.com')
+        );
+
+        $lastHitDomain = null;
+        $buildInformationService->method('updateLastHit')->willReturnCallback(
+            static function (string $domain) use (&$lastHitDomain): void {
+                $lastHitDomain = $domain;
+            }
+        );
+
+        $quarantinedDomain = null;
+        $quarantineService = $this->createMock(DocumentationQuarantineService::class);
+        $quarantineService->method('isQuarantined')->willReturn(false);
+        $quarantineService->method('quarantine')->willReturnCallback(
+            static function (PushEvent $event, string $domain) use (&$quarantinedDomain): DocumentationQuarantine {
+                $quarantinedDomain = $domain;
+
+                return new DocumentationQuarantine();
+            }
+        );
+
+        $subject = new RenderDocumentationService(
+            $buildInformationService,
+            $this->createMock(GithubService::class),
+            new HistoryService($this->createMock(EntityManagerInterface::class)),
+            new NullLogger(),
+            $quarantineService,
+            $this->createMock(RepositoryBlacklistEntryRepository::class),
+            $this->createMock(MailService::class),
+            $this->createMock(Security::class),
+        );
+
+        try {
+            $subject->requestDocumentationRendering($pushEvent, DocumentationRenderingTrigger::API);
+            $this->fail('An unknown domain has to decline the rendering request.');
+        } catch (DocumentationRenderingRequestDeclinedException) {
+            // expected, the assertions below are what this test is about
+        }
+
+        $this->assertSame('raw.githubusercontent.com', $quarantinedDomain, 'The quarantine has to record the host the request was blocked on.');
+        $this->assertSame('raw.githubusercontent.com', $lastHitDomain, 'The last hit belongs to the domain row the check looks up.');
+    }
+}