fix: store long advisories and mark skipped ones on results

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- [#175](https://github.com/itk-dev/devops_itksites/pull/175)
+  Store long advisory constraints, titles and links in full, so one long
+  advisory no longer blocks the others, and mark detection results with the
+  advisories they could not store
 - [#174](https://github.com/itk-dev/devops_itksites/pull/174)
   Show detection result errors and warnings as readable messages instead of
   JSON, so long messages are no longer cut off

migrations/Version20261104214553.php

@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20261104214553 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Store long advisory values as text, and why parts of a detection result could not be processed';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE advisory CHANGE affected_versions affected_versions LONGTEXT NOT NULL, CHANGE title title LONGTEXT NOT NULL, CHANGE link link LONGTEXT NOT NULL');
+        // Existing rows get an empty list before the column becomes NOT NULL.
+        $this->addSql('ALTER TABLE detection_result ADD processing_errors JSON DEFAULT NULL');
+        $this->addSql("UPDATE detection_result SET processing_errors = '[]'");
+        $this->addSql('ALTER TABLE detection_result MODIFY processing_errors JSON NOT NULL');
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE advisory CHANGE affected_versions affected_versions VARCHAR(500) NOT NULL, CHANGE title title VARCHAR(255) NOT NULL, CHANGE link link VARCHAR(255) NOT NULL');
+        $this->addSql('ALTER TABLE detection_result DROP processing_errors');
+    }
+
+    public function isTransactional(): bool
+    {
+        return false;
+    }
+}

src/Controller/Admin/DetectionResultCrudController.php

@@ -59,6 +59,8 @@ public function configureFields(string $pageName): iterable
             ->setTemplatePath('EasyAdminBundle/Fields/harvester_messages.html.twig');
         yield Field::new('warnings')->onlyOnDetail()
             ->setTemplatePath('EasyAdminBundle/Fields/harvester_messages.html.twig');
+        yield Field::new('processingErrors')->onlyOnDetail()
+            ->setTemplatePath('EasyAdminBundle/Fields/sources.html.twig');
         yield Field::new('prettyData')->onlyOnDetail()->setLabel('Data')
             ->setTemplatePath('EasyAdminBundle/Fields/code_highlight.html.twig')
             ->setCustomOption('language', 'json');

src/Entity/Advisory.php

@@ -9,6 +9,7 @@
 use Doctrine\Common\Collections\Collection;
 use Doctrine\DBAL\Types\Types;
 use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Validator\Constraints as Assert;
 
 #[ORM\Entity(repositoryClass: AdvisoryRepository::class)]
 class Advisory extends AbstractBaseEntity implements \Stringable
@@ -20,18 +21,20 @@ class Advisory extends AbstractBaseEntity implements \Stringable
     private const string DRUPAL_ADVISORY_URL_PATTERN = 'https://www.drupal.org/%s';
 
     #[ORM\Column(length: 255, unique: true)]
+    #[Assert\Length(max: 255)]
     private ?string $advisoryId = null;
 
-    #[ORM\Column(length: 500)]
+    #[ORM\Column(type: Types::TEXT)]
     private ?string $affectedVersions = null;
 
-    #[ORM\Column(length: 255)]
+    #[ORM\Column(type: Types::TEXT)]
     private ?string $title = null;
 
     #[ORM\Column(length: 50, nullable: true)]
+    #[Assert\Length(max: 50)]
     private ?string $cve = null;
 
-    #[ORM\Column(length: 255)]
+    #[ORM\Column(type: Types::TEXT)]
     private ?string $link = null;
 
     #[ORM\Column]

src/Entity/DetectionResult.php

@@ -108,6 +108,14 @@ class DetectionResult extends AbstractBaseEntity implements \Stringable
     )]
     private array $warnings = [];
 
+    /**
+     * Why parts of the result could not be processed, e.g. advisories that were skipped. Set by ITKsites, not the harvester.
+     *
+     * @var list<string>
+     */
+    #[ORM\Column(type: Types::JSON)]
+    private array $processingErrors = [];
+
     #[ORM\Column(type: Types::STRING, length: 255, unique: true)]
     private string $hash;
 
@@ -219,6 +227,27 @@ public function setWarnings(array $warnings): self
         return $this;
     }
 
+    /** @return list<string> */
+    public function getProcessingErrors(): array
+    {
+        return $this->processingErrors;
+    }
+
+    /** @param list<string> $processingErrors */
+    public function setProcessingErrors(array $processingErrors): self
+    {
+        $this->processingErrors = $processingErrors;
+
+        return $this;
+    }
+
+    public function addProcessingError(string $processingError): self
+    {
+        $this->processingErrors[] = $processingError;
+
+        return $this;
+    }
+
     public function getHash(): ?string
     {
         return $this->hash;

src/Handler/DockerImageHandler.php

@@ -61,7 +61,9 @@ public function handleResult(DetectionResult $detectionResult): void
                     $this->packageVersionFactory->setPackageVersions($site->getInstallation(), $container->packages->installed);
                 }
                 if (isset($container->audit) && is_object($container->audit)) {
-                    $this->advisoryFactory->setAdvisories($site->getInstallation(), $container->audit);
+                    foreach ($this->advisoryFactory->setAdvisories($site->getInstallation(), $container->audit) as $skipped) {
+                        $detectionResult->addProcessingError($skipped);
+                    }
                 }
                 if (isset($container->drupal) && is_object($container->drupal)) {
                     $this->moduleVersionFactory->setModuleVersions($site->getInstallation(), $container->drupal);

src/MessageHandler/ProcessDetectionResultHandler.php

@@ -26,6 +26,9 @@ public function __invoke(ProcessDetectionResult $message): void
         $detectionResult = $this->detectionResultRepository->find($message->detectionResultId);
 
         if (null !== $detectionResult) {
+            // Each run records its own problems, so a replay shows the current state.
+            $detectionResult->setProcessingErrors([]);
+
             /** @var DetectionResultHandlerInterface $handler */
             foreach ($this->resultHandlers as $handler) {
                 if ($handler->supportsType($detectionResult->getType())) {

src/Service/AdvisoryFactory.php

@@ -12,18 +12,24 @@
 use Composer\Semver\Semver;
 use Doctrine\ORM\EntityManagerInterface;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Validator\Validator\ValidatorInterface;
 
 class AdvisoryFactory
 {
     public function __construct(
         private readonly EntityManagerInterface $entityManager,
         private readonly AdvisoryRepository $advisoryRepository,
         private readonly LoggerInterface $logger,
+        private readonly ValidatorInterface $validator,
     ) {
     }
 
-    public function setAdvisories(Installation $installation, object $audit): void
+    /**
+     * @return list<string> why advisories of the audit were skipped, for the detection result
+     */
+    public function setAdvisories(Installation $installation, object $audit): array
     {
+        $skipped = [];
         if (isset($audit->advisories) && is_object($audit->advisories)) {
             foreach ($audit->advisories as $package => $advisories) {
                 foreach ($advisories as $advisory) {
@@ -36,15 +42,13 @@ public function setAdvisories(Installation $installation, object $audit): void
                             'package' => $advisory->packageName,
                             'installation' => (string) $installation,
                         ]);
+                        $skipped[] = sprintf('Skipped advisory %s for %s, which the installation does not list', $advisory->advisoryId, $advisory->packageName);
                         continue;
                     }
                     $entity = $this->advisoryRepository->findOneBy(['advisoryId' => $advisory->advisoryId]);
 
                     if (null === $entity) {
                         $entity = new Advisory();
-                        $this->entityManager->persist($entity);
-
-                        $entity->addPackageVersion($packageVersion);
 
                         if (isset($advisory->advisoryId)) {
                             $entity->setAdvisoryId($advisory->advisoryId);
@@ -55,9 +59,6 @@ public function setAdvisories(Installation $installation, object $audit): void
                         if (isset($advisory->title)) {
                             $entity->setTitle($advisory->title);
                         }
-                        if (isset($advisory->title)) {
-                            $entity->setTitle($advisory->title);
-                        }
                         if (isset($advisory->cve)) {
                             $entity->setCve($advisory->cve);
                         }
@@ -75,12 +76,44 @@ public function setAdvisories(Installation $installation, object $audit): void
                         if (!empty($advisory->sources)) {
                             $entity->setSources($advisory->sources);
                         }
+
+                        // Persisted with the rest of the result, so one that does not fit would fail them all.
+                        $reason = $this->unstorableReason($entity);
+                        if (null !== $reason) {
+                            $skipped[] = $reason;
+                            continue;
+                        }
+
+                        $this->entityManager->persist($entity);
+                        $entity->addPackageVersion($packageVersion);
                     }
 
                     $this->setAdvisoryForAffectedVersions($packageVersion->getPackage(), $entity);
                 }
             }
         }
+
+        return $skipped;
+    }
+
+    /**
+     * Why the advisory does not fit the database, or null when it does. Only
+     * the advisory id and CVE have a fixed size, so only values that cannot be
+     * real ids are refused. Logged as an error: prod writes nothing below it.
+     */
+    public function unstorableReason(Advisory $advisory): ?string
+    {
+        $violations = $this->validator->validate($advisory);
+        if (0 === count($violations)) {
+            return null;
+        }
+
+        $this->logger->error('Skipped advisory {advisory}, which does not fit the database: {violations}', [
+            'advisory' => $advisory->getAdvisoryId(),
+            'violations' => (string) $violations,
+        ]);
+
+        return sprintf('Skipped advisory %s, which does not fit the database: %s', $advisory->getAdvisoryId(), trim((string) $violations));
     }
 
     private function getInstalledPackageVersion(Installation $installation, string $vendorPackage): ?PackageVersion

src/Service/Drupal/ReleaseChecker.php

@@ -64,7 +64,9 @@ public function check(Package $package): void
                 $constraint = DrupalVersion::constraintToComposer($securityAdvisory->affectedVersions);
                 if ($this->matchesAny($insecureVersions, $constraint)) {
                     $advisory = $this->upsert($package, $securityAdvisory, $constraint);
-                    $this->advisoryFactory->setAdvisoryForAffectedVersions($package, $advisory);
+                    if (null !== $advisory) {
+                        $this->advisoryFactory->setAdvisoryForAffectedVersions($package, $advisory);
+                    }
                 }
             }
         }
@@ -90,14 +92,13 @@ private function matchesAny(array $versions, string $constraint): bool
         return false;
     }
 
-    private function upsert(Package $package, SecurityAdvisory $securityAdvisory, string $constraint): Advisory
+    /**
+     * Null when the advisory does not fit the database; flushing it would fail the whole package.
+     */
+    private function upsert(Package $package, SecurityAdvisory $securityAdvisory, string $constraint): ?Advisory
     {
-        $advisory = $this->advisoryRepository->findOneBy(['advisoryId' => $securityAdvisory->advisoryId]);
-        if (null === $advisory) {
-            $advisory = new Advisory()->setAdvisoryId($securityAdvisory->advisoryId);
-            $this->entityManager->persist($advisory);
-            $package->addAdvisory($advisory);
-        }
+        $existing = $this->advisoryRepository->findOneBy(['advisoryId' => $securityAdvisory->advisoryId]);
+        $advisory = $existing ?? new Advisory()->setAdvisoryId($securityAdvisory->advisoryId);
 
         $advisory
             ->setAffectedVersions($constraint)
@@ -109,6 +110,20 @@ private function upsert(Package $package, SecurityAdvisory $securityAdvisory, st
             $advisory->setCve($securityAdvisory->cve);
         }
 
+        if (null !== $this->advisoryFactory->unstorableReason($advisory)) {
+            if (null !== $existing) {
+                // Drop the changes so the stored advisory is flushed unchanged.
+                $this->entityManager->refresh($existing);
+            }
+
+            return null;
+        }
+
+        if (null === $existing) {
+            $this->entityManager->persist($advisory);
+            $package->addAdvisory($advisory);
+        }
+
         return $advisory;
     }
 }