Merge branch 'release/7.3.0'

Author: andreaskienast

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
       - uses: actions/checkout@v7
 
       - name: Setup node
-        uses: actions/setup-node@v6
+        uses: actions/setup-node@v7
         with:
           node-version-file: '.nvmrc'
 
@@ -110,7 +110,7 @@ jobs:
       - uses: actions/checkout@v7
 
       - name: Setup node
-        uses: actions/setup-node@v6
+        uses: actions/setup-node@v7
         with:
           node-version-file: '.nvmrc'
 

legacy_hook/composer.lock

@@ -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;
+    }
+}

migrations/Version20260804120000.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);
                 }

package-lock.json

@@ -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/Controller/AdminInterface/Docs/KnownRepositoryDomainsController.php

@@ -18,6 +18,7 @@ final class DocsRenderingHistoryStatus
     public const NO_COMPOSER_JSON = 'noComposerJson';
     public const INVALID_COMPOSER_JSON = 'invalidComposerJson';
     public const UNKNOWN_REPOSITORY_DOMAIN = 'unknownRepositoryDomain';
+    public const INVALID_COMPOSER_JSON_URL = 'invalidComposerJsonUrl';
     public const PACKAGE_REGISTERED_WITH_DIFFERENT_REPOSITORY = 'packageRegisteredWithDifferentRepository';
     public const NO_RELEVANT_BRANCH_OR_TAG = 'noRelevantBranchOrTag';
     public const MISSING_VALUE_IN_COMPOSER_JSON = 'missingValueInComposerJson';
@@ -35,6 +36,7 @@ final class DocsRenderingHistoryStatus
         self::NO_COMPOSER_JSON,
         self::INVALID_COMPOSER_JSON,
         self::UNKNOWN_REPOSITORY_DOMAIN,
+        self::INVALID_COMPOSER_JSON_URL,
         self::PACKAGE_REGISTERED_WITH_DIFFERENT_REPOSITORY,
         self::NO_RELEVANT_BRANCH_OR_TAG,
         self::MISSING_VALUE_IN_COMPOSER_JSON,
@@ -55,6 +57,7 @@ final class DocsRenderingHistoryStatus
         self::NO_COMPOSER_JSON => 'No composer.json found.',
         self::INVALID_COMPOSER_JSON => 'Invalid composer.json.',
         self::UNKNOWN_REPOSITORY_DOMAIN => 'Unknown repository domain.',
+        self::INVALID_COMPOSER_JSON_URL => 'The composer.json url can not be used.',
         self::PACKAGE_REGISTERED_WITH_DIFFERENT_REPOSITORY => 'Package registered with different repository.',
         self::NO_RELEVANT_BRANCH_OR_TAG => 'No relevant branch or tag found.',
         self::MISSING_VALUE_IN_COMPOSER_JSON => 'Missing value in composer.json.',

src/Controller/AdminInterface/Docs/QuarantinedDocumentationsController.php

@@ -33,6 +33,9 @@
 use GuzzleHttp\ClientInterface;
 use GuzzleHttp\Exception\GuzzleException;
 use GuzzleHttp\Psr7\Uri;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\UriInterface;
 use Symfony\Component\Filesystem\Filesystem;
 
 /**
@@ -75,7 +78,20 @@ public function fetchRemoteComposerJson(string $path): array
         $this->assertUrlToComposerFileIsSafe($path);
 
         try {
-            $response = $this->generalClient->request('GET', $path);
+            // The url is only known to be safe until the first redirect, so every
+            // hop has to pass the same check. Without this an open redirect on an
+            // allowed domain would be enough to reach an arbitrary target.
+            $response = $this->generalClient->request('GET', $path, [
+                'allow_redirects' => [
+                    'max' => 5,
+                    'protocols' => ['http', 'https'],
+                    'strict' => false,
+                    'referer' => false,
+                    'on_redirect' => function (RequestInterface $request, ResponseInterface $response, UriInterface $uri): void {
+                        $this->assertUrlToComposerFileIsSafe((string) $uri);
+                    },
+                ],
+            ]);
         } catch (GuzzleException $e) {
             throw new ComposerJsonNotFoundException($e->getMessage(), $e->getCode());
         }
@@ -324,6 +340,14 @@ private function assertUrlToComposerFileIsSafe(string $url): void
             throw new InvalidComposerJsonUrlException('URL to composer.json contains disallowed scheme', 1781613532, null, $url);
         }
 
+        // The url is assembled from payload fields, so a '#' or a '?' inside one of
+        // them can push the intended '…/composer.json' suffix out of the path and
+        // leave an arbitrary endpoint on the same host. Every format this
+        // application builds ends in that suffix, so require it.
+        if (!str_ends_with($uri->getPath(), '/composer.json')) {
+            throw new InvalidComposerJsonUrlException('URL to composer.json does not point to a composer.json', 1785816000, null, $url);
+        }
+
         $normalizedHost = RepositoryUrlUtility::getNormalizedDomain($uri);
         $allowedRepositoryDomain = $this->knownRepositoryDomainsRepository->findOneBy([
             'domain' => $normalizedHost,

src/Enum/DocsRenderingHistoryStatus.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/DocumentationBuildInformationService.php

@@ -20,6 +20,7 @@ class GitRepositoryService
     final public const SERVICE_BITBUCKET_SERVER = 'bitbucket-server';
     final public const SERVICE_GITHUB = 'github';
     final public const SERVICE_GITLAB = 'gitlab';
+    final public const SERVICE_FORGEJO = 'forgejo';
 
     final public const SERVICE_NAMES = [
         self::SERVICE_GITHUB => 'GitHub',
@@ -32,6 +33,7 @@ class GitRepositoryService
         self::SERVICE_BITBUCKET_SERVER => '{baseUrl}/projects/{project}/repos/{package}/raw/composer.json?at=refs%2F{type}%2F{version}',
         self::SERVICE_GITLAB => '{baseUrl}/raw/{version}/composer.json',
         self::SERVICE_GITHUB => 'https://raw.githubusercontent.com/{repoName}/{version}/composer.json',
+        self::SERVICE_FORGEJO => '{baseUrl}/raw/{type}/{version}/composer.json',
     ];
     protected array $allowedBranches = ['master', 'main', 'documentation-draft'];
 
@@ -41,6 +43,7 @@ public function resolvePublicComposerJsonUrlByPayload(\stdClass $payload, string
             self::SERVICE_BITBUCKET_SERVER, self::SERVICE_BITBUCKET_CLOUD => $this->getPublicComposerUrlForBitbucket($payload),
             self::SERVICE_GITHUB => $this->getPublicComposerUrlForGithub($payload),
             self::SERVICE_GITLAB => $this->getPublicComposerUrlForGitlab($payload),
+            self::SERVICE_FORGEJO => $this->getPublicComposerUrlForForgejo($payload),
             default => '',
         };
     }
@@ -139,6 +142,18 @@ protected function getPublicComposerUrlForGitlab(\stdClass $payload): string
         ]);
     }
 
+    protected function getPublicComposerUrlForForgejo(\stdClass $payload): string
+    {
+        $ref = (string) $payload->ref;
+        $version = str_replace(['refs/tags/', 'refs/heads/'], '', $ref);
+
+        return $this->getParsedUrl($this->composerJsonUrlFormat[self::SERVICE_FORGEJO], [
+            '{baseUrl}' => (string) $payload->repository->html_url,
+            '{type}' => str_starts_with($ref, 'refs/tags/') ? 'tag' : 'branch',
+            '{version}' => $version,
+        ]);
+    }
+
     protected function getPublicComposerUrlForGithub(\stdClass $payload): string
     {
         $version = str_replace(['refs/tags/', 'refs/heads/'], '', (string) $payload->ref);