diff --git a/src/Enum/DocsRenderingHistoryStatus.php b/src/Enum/DocsRenderingHistoryStatus.php index 5a33ebb..edd9746 100644 --- a/src/Enum/DocsRenderingHistoryStatus.php +++ b/src/Enum/DocsRenderingHistoryStatus.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.', diff --git a/src/Service/DocumentationBuildInformationService.php b/src/Service/DocumentationBuildInformationService.php index ce3ea8c..8e285a0 100644 --- a/src/Service/DocumentationBuildInformationService.php +++ b/src/Service/DocumentationBuildInformationService.php @@ -324,6 +324,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, diff --git a/src/Service/RenderDocumentationService.php b/src/Service/RenderDocumentationService.php index bdaa161..5b9594b 100644 --- a/src/Service/RenderDocumentationService.php +++ b/src/Service/RenderDocumentationService.php @@ -26,6 +26,7 @@ use App\Exception\DocsPackageDoNotCareBranch; use App\Exception\DocsPackageRegisteredWithDifferentRepositoryException; use App\Exception\DocumentationRenderingRequestDeclinedException; +use App\Exception\InvalidComposerJsonUrlException; use App\Exception\UnknownComposerJsonUrlException; use App\Extractor\DeploymentInformation; use App\Extractor\PushEvent; @@ -90,6 +91,20 @@ public function requestDocumentationRendering(PushEvent $pushEvent, Documentatio )); throw new DocumentationRenderingRequestDeclinedException(sprintf('composer.json\'s host domain %s is disallowed for rendering request', $e->normalizedHost), 1782294348, $e); + } catch (InvalidComposerJsonUrlException $e) { + $this->historyService->writeHistory(new HistoryEntryDto( + type: HistoryEntryType::DOCS_RENDERING, + status: DocsRenderingHistoryStatus::INVALID_COMPOSER_JSON_URL, + triggeredBy: $trigger->toHistoryEntryTrigger(), + data: [ + 'repository' => $pushEvent->getRepositoryUrl(), + 'composerFile' => $pushEvent->getUrlToComposerFile(), + 'payload' => $pushEvent->getPayload(), + 'user' => $userIdentifier, + ] + )); + + throw new DocumentationRenderingRequestDeclinedException(sprintf('composer.json url %s can not be used for a rendering request', $e->composerJsonUrl), 1785810600, $e); } $composerAsObject = $this->documentationBuildInformationService->getComposerJsonObject($composerJson); diff --git a/tests/Unit/Service/ComposerJsonUrlShapeTest.php b/tests/Unit/Service/ComposerJsonUrlShapeTest.php new file mode 100644 index 0000000..c854999 --- /dev/null +++ b/tests/Unit/Service/ComposerJsonUrlShapeTest.php @@ -0,0 +1,87 @@ + ['https://allowed.example/acme/ext/raw/main/composer.json']; + yield 'bitbucket server' => ['https://allowed.example/projects/EXT/repos/ext/raw/composer.json?at=refs%2Fheads%2Fmain']; + yield 'gitlab' => ['https://allowed.example/acme/ext/raw/main/composer.json']; + yield 'github' => ['https://allowed.example/acme/ext/main/composer.json']; + yield 'forgejo' => ['https://allowed.example/acme/ext/raw/branch/main/composer.json']; + } + + #[DataProvider('urlsBuiltForTheSupportedServicesDataProvider')] + public function testUrlsTheServicesActuallyProduceArePassed(string $url): void + { + $composerJson = $this->buildSubject()->fetchRemoteComposerJson($url); + + $this->assertSame('acme/ext', $composerJson['name']); + } + + public static function manipulatedUrlsDataProvider(): \Iterator + { + // A '#' turns the expected '/…/composer.json' suffix into a fragment, + // which is dropped before the request is sent + yield 'fragment cuts off the expected path' => ['https://allowed.example/internal/admin#/raw/branch/main/composer.json']; + // A '?' in the base url pushes the expected suffix into the query string + yield 'query swallows the expected path' => ['https://allowed.example/api/v4/user?a=/raw/branch/main/composer.json']; + yield 'path traversal out of the repository' => ['https://allowed.example/acme/ext/raw/branch/../../../../etc/passwd']; + yield 'no composer.json at all' => ['https://allowed.example/acme/ext/raw/branch/main/']; + } + + #[DataProvider('manipulatedUrlsDataProvider')] + public function testUrlsNotPointingAtAComposerJsonAreRejected(string $url): void + { + $this->expectException(InvalidComposerJsonUrlException::class); + + $this->buildSubject()->fetchRemoteComposerJson($url); + } + + private function buildSubject(): DocumentationBuildInformationService + { + $knownDomain = (new KnownRepositoryDomain())->setDomain('allowed.example')->setStatus(RepositoryDomainStatus::ALLOWED); + $knownRepositoryDomainRepository = $this->createMock(KnownRepositoryDomainRepository::class); + $knownRepositoryDomainRepository->method('findOneBy')->willReturn($knownDomain); + + return new DocumentationBuildInformationService( + '/tmp', + 'sub', + $this->createMock(DocumentationJarRepository::class), + $knownRepositoryDomainRepository, + $this->createMock(EntityManagerInterface::class), + $this->createMock(Filesystem::class), + new Client(['handler' => HandlerStack::create(new MockHandler([new Response(200, [], '{"name": "acme/ext"}')]))]), + $this->createMock(SlackService::class), + $this->createMock(MailService::class), + ); + } +}