From 638e4337506e2ca8974773b993c1e4655a723674 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 16:37:35 +0200 Subject: [PATCH 01/15] feat: mark advisories as not affected per git repo On a site's detail page, an advisory can be marked as not affected with a reason, one at a time or all current ones at once, and a mark can be removed. A mark belongs to the git repo, so it covers every checkout of it, and sites without a repo cannot be marked. Marked advisories no longer count on the site list, in its sort or on the repo advisories page. Marks are stored by repo and advisory name, so truncating derived data and replays leave them in place. --- fixtures/package.yaml | 10 + migrations/Version20261021140107.php | 26 +++ src/Admin/AdvisoryCountSort.php | 8 +- .../Admin/AdvisoryAssessmentController.php | 79 +++++++ src/Controller/Admin/SiteCrudController.php | 14 +- src/Entity/AdvisoryAssessment.php | 106 +++++++++ .../AdvisoryAssessmentRepository.php | 45 ++++ src/Repository/GitRepoRepository.php | 7 +- src/Service/AdvisoryAssessor.php | 99 +++++++++ .../Fields/site_advisories.html.twig | 78 +++++++ .../AdvisoryAssessmentControllerTest.php | 207 ++++++++++++++++++ tests/DataFixtures/FixturesTest.php | 2 + tests/Service/AdvisoryAssessorTest.php | 113 ++++++++++ 13 files changed, 789 insertions(+), 5 deletions(-) create mode 100644 migrations/Version20261021140107.php create mode 100644 src/Controller/Admin/AdvisoryAssessmentController.php create mode 100644 src/Entity/AdvisoryAssessment.php create mode 100644 src/Repository/AdvisoryAssessmentRepository.php create mode 100644 src/Service/AdvisoryAssessor.php create mode 100644 templates/EasyAdminBundle/Fields/site_advisories.html.twig create mode 100644 tests/Controller/Admin/AdvisoryAssessmentControllerTest.php create mode 100644 tests/Service/AdvisoryAssessorTest.php diff --git a/fixtures/package.yaml b/fixtures/package.yaml index 0adca1db..8a589cf1 100644 --- a/fixtures/package.yaml +++ b/fixtures/package.yaml @@ -6,6 +6,7 @@ # - drupal/core has a drupal.org and a Packagist advisory for the same CVE. # - league/commonmark and drupal/webform have advisories without a CVE. # - guzzle has no advisories. +# - example-org/intranet has the commonmark advisory marked as not affected. App\Entity\Package: package/symfony-http-foundation: @@ -106,6 +107,15 @@ App\Entity\Advisory: - name: "Drupal" remoteId: "SA-CONTRIB-2099-001" +# Advisories a developer has marked as not affected, per git repo. +App\Entity\AdvisoryAssessment: + advisory_assessment/intranet-commonmark: + advisoryId: "PKSA-m3r8-5tyv-1h6p" + reason: "Markdown is only rendered from editor input" + assessedBy: "user@example.com" + __calls: + - setGitRepo: ["@git_repo/intranet"] + App\Entity\PackageVersion: package_version/symfony-http-foundation-6.4.10: package: "@package/symfony-http-foundation" diff --git a/migrations/Version20261021140107.php b/migrations/Version20261021140107.php new file mode 100644 index 00000000..93981a7e --- /dev/null +++ b/migrations/Version20261021140107.php @@ -0,0 +1,26 @@ +addSql('CREATE TABLE advisory_assessment (id BINARY(16) NOT NULL, created_at DATETIME NOT NULL, modified_at DATETIME NOT NULL, created_by VARCHAR(255) DEFAULT \'\' NOT NULL, modified_by VARCHAR(255) DEFAULT \'\' NOT NULL, repo_provider VARCHAR(255) NOT NULL, repo_organization VARCHAR(255) NOT NULL, repo_name VARCHAR(255) NOT NULL, advisory_id VARCHAR(255) NOT NULL, reason LONGTEXT NOT NULL, assessed_by VARCHAR(180) NOT NULL, UNIQUE INDEX repo_advisory (repo_provider, repo_organization, repo_name, advisory_id), PRIMARY KEY (id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci`'); + } + + public function down(Schema $schema): void + { + $this->addSql('DROP TABLE advisory_assessment'); + } +} diff --git a/src/Admin/AdvisoryCountSort.php b/src/Admin/AdvisoryCountSort.php index 5812d5e5..0d379c77 100644 --- a/src/Admin/AdvisoryCountSort.php +++ b/src/Admin/AdvisoryCountSort.php @@ -5,6 +5,8 @@ namespace App\Admin; use App\Entity\Advisory; +use App\Entity\AdvisoryAssessment; +use App\Entity\GitRepo; use Doctrine\ORM\QueryBuilder; use EasyCorp\Bundle\EasyAdminBundle\Config\Option\SortOrder; use EasyCorp\Bundle\EasyAdminBundle\Dto\SearchDto; @@ -34,10 +36,14 @@ public static function apply(QueryBuilder $qb, SearchDto $searchDto): void // An advisory belongs to a single package and an installation holds a // single version of it, so the advisory cannot be counted twice. + // Advisories marked as not affected for the installation's git repo + // do not count, as in AdvisoryAssessor::openCount(). $qb->addSelect(sprintf( - '(SELECT COUNT(adv.id) FROM %s adv JOIN adv.packageVersions advPackageVersion JOIN advPackageVersion.installations advInstallation WHERE advInstallation.id = IDENTITY(%s.installation)) AS HIDDEN %s', + '(SELECT COUNT(adv.id) FROM %1$s adv JOIN adv.packageVersions advPackageVersion JOIN advPackageVersion.installations advInstallation WHERE advInstallation.id = IDENTITY(%2$s.installation) AND NOT EXISTS (SELECT advAssessment.id FROM %3$s advAssessment JOIN %4$s advRepo WITH advRepo.provider = advAssessment.repoProvider AND advRepo.organization = advAssessment.repoOrganization AND advRepo.repo = advAssessment.repoName JOIN advRepo.gitTags advTag JOIN advTag.installations advTagInstallation WHERE advTagInstallation.id = advInstallation.id AND advAssessment.advisoryId = adv.advisoryId)) AS HIDDEN %5$s', Advisory::class, $rootAlias, + AdvisoryAssessment::class, + GitRepo::class, self::PROPERTY )); diff --git a/src/Controller/Admin/AdvisoryAssessmentController.php b/src/Controller/Admin/AdvisoryAssessmentController.php new file mode 100644 index 00000000..3d6870d3 --- /dev/null +++ b/src/Controller/Admin/AdvisoryAssessmentController.php @@ -0,0 +1,79 @@ +getId(); + } + + #[AdminRoute(path: '/site/{id}/advisories/not-affected', name: 'site_advisories_not_affected', options: ['methods' => ['POST']])] + public function markNotAffected(#[MapEntity(id: 'id')] Site $site, Request $request): RedirectResponse + { + $this->denyUnlessTokenIsValid($site, $request); + + try { + $marked = $this->assessor->markNotAffected( + $site, + array_values(array_map(strval(...), $request->request->all('advisoryIds'))), + $request->request->getString('reason'), + (string) $this->getUser()?->getUserIdentifier(), + ); + $this->addFlash('success', sprintf( + 'Marked %d %s as not affected for all checkouts of %s.', + $marked, + 1 === $marked ? 'advisory' : 'advisories', + $this->assessor->gitRepo($site), + )); + } catch (\InvalidArgumentException $exception) { + $this->addFlash('danger', $exception->getMessage()); + } + + return $this->redirectToSite($site); + } + + #[AdminRoute(path: '/site/{id}/advisories/remove-mark', name: 'site_advisories_remove_mark', options: ['methods' => ['POST']])] + public function removeMark(#[MapEntity(id: 'id')] Site $site, Request $request): RedirectResponse + { + $this->denyUnlessTokenIsValid($site, $request); + + $advisoryId = $request->request->getString('advisoryId'); + $this->assessor->removeMark($site, $advisoryId); + $this->addFlash('success', sprintf('Removed the mark on %s, so it counts again.', $advisoryId)); + + return $this->redirectToSite($site); + } + + private function denyUnlessTokenIsValid(Site $site, Request $request): void + { + if (!$this->isCsrfTokenValid(self::csrfIntent($site), $request->request->getString('_token'))) { + throw $this->createAccessDeniedException('Invalid CSRF token.'); + } + } + + private function redirectToSite(Site $site): RedirectResponse + { + return $this->redirectToRoute('admin_site_detail', ['entityId' => $site->getId()]); + } +} diff --git a/src/Controller/Admin/SiteCrudController.php b/src/Controller/Admin/SiteCrudController.php index 5c333cb4..8665aee4 100644 --- a/src/Controller/Admin/SiteCrudController.php +++ b/src/Controller/Admin/SiteCrudController.php @@ -16,6 +16,7 @@ use App\Form\Type\Admin\HostingProviderFilter; use App\Form\Type\Admin\SemverFilter; use App\Form\Type\Admin\ServerTypeFilter; +use App\Service\AdvisoryAssessor; use App\Trait\ExportCrudControllerTrait; use App\Trait\SemverSortableCrudControllerTrait; use Doctrine\ORM\QueryBuilder; @@ -43,8 +44,9 @@ class SiteCrudController extends AbstractCrudController createIndexQueryBuilder as private semverIndexQueryBuilder; } - public function __construct() - { + public function __construct( + private readonly AdvisoryAssessor $assessor, + ) { } public static function getEntityFqcn(): string @@ -71,7 +73,13 @@ public function configureActions(Actions $actions): Actions public function configureFields(string $pageName): iterable { yield DomainField::new('primaryDomain')->setColumns(12); - yield AdvisoryCountField::new('advisoryCount')->setLabel('Adv.')->setSortable(true); + // Advisories marked as not affected do not count. The detail page + // lists them all, with their marks. + yield AdvisoryCountField::new('advisoryCount')->setLabel('Adv.')->setSortable(true) + ->setTemplatePath('EasyAdminBundle/Fields/site_advisories.html.twig') + ->formatValue(Crud::PAGE_DETAIL === $pageName + ? fn (mixed $value, Site $site): array => ['gitRepo' => $this->assessor->gitRepo($site), 'advisories' => $this->assessor->advisories($site)] + : fn (mixed $value, Site $site): int => $this->assessor->openCount($site)); yield AssociationField::new('domains')->hideOnIndex(); yield SiteTypeField::new('type')->setLabel('Stack'); yield ConfigFilePathField::new('configFilePath')->setColumns(12)->hideOnIndex(); diff --git a/src/Entity/AdvisoryAssessment.php b/src/Entity/AdvisoryAssessment.php new file mode 100644 index 00000000..8ce0be40 --- /dev/null +++ b/src/Entity/AdvisoryAssessment.php @@ -0,0 +1,106 @@ +repoOrganization.'/'.$this->repoName.': '.$this->advisoryId; + } + + public function setGitRepo(GitRepo $gitRepo): self + { + $this->repoProvider = (string) $gitRepo->getProvider(); + $this->repoOrganization = (string) $gitRepo->getOrganization(); + $this->repoName = (string) $gitRepo->getRepo(); + + return $this; + } + + public function getRepoProvider(): string + { + return $this->repoProvider; + } + + public function getRepoOrganization(): string + { + return $this->repoOrganization; + } + + public function getRepoName(): string + { + return $this->repoName; + } + + public function getAdvisoryId(): string + { + return $this->advisoryId; + } + + public function setAdvisoryId(string $advisoryId): self + { + $this->advisoryId = $advisoryId; + + return $this; + } + + public function getReason(): string + { + return $this->reason; + } + + public function setReason(string $reason): self + { + $this->reason = $reason; + + return $this; + } + + public function getAssessedBy(): string + { + return $this->assessedBy; + } + + public function setAssessedBy(string $assessedBy): self + { + $this->assessedBy = $assessedBy; + + return $this; + } +} diff --git a/src/Repository/AdvisoryAssessmentRepository.php b/src/Repository/AdvisoryAssessmentRepository.php new file mode 100644 index 00000000..76cf0804 --- /dev/null +++ b/src/Repository/AdvisoryAssessmentRepository.php @@ -0,0 +1,45 @@ + + * + * @method AdvisoryAssessment|null find($id, $lockMode = null, $lockVersion = null) + * @method AdvisoryAssessment|null findOneBy(array $criteria, array $orderBy = null) + * @method AdvisoryAssessment[] findAll() + * @method AdvisoryAssessment[] findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null) + */ +class AdvisoryAssessmentRepository extends ServiceEntityRepository +{ + public function __construct(ManagerRegistry $registry) + { + parent::__construct($registry, AdvisoryAssessment::class); + } + + /** + * The repo's assessments, keyed by advisory id. + * + * @return array + */ + public function findByGitRepo(GitRepo $gitRepo): array + { + $assessments = []; + foreach ($this->findBy([ + 'repoProvider' => $gitRepo->getProvider(), + 'repoOrganization' => $gitRepo->getOrganization(), + 'repoName' => $gitRepo->getRepo(), + ]) as $assessment) { + $assessments[$assessment->getAdvisoryId()] = $assessment; + } + + return $assessments; + } +} diff --git a/src/Repository/GitRepoRepository.php b/src/Repository/GitRepoRepository.php index d1dae337..eba1bb9e 100644 --- a/src/Repository/GitRepoRepository.php +++ b/src/Repository/GitRepoRepository.php @@ -4,8 +4,10 @@ namespace App\Repository; +use App\Entity\AdvisoryAssessment; use App\Entity\GitRepo; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; +use Doctrine\ORM\Query\Expr\Join; use Doctrine\Persistence\ManagerRegistry; use Symfony\Component\Uid\Ulid; @@ -25,7 +27,8 @@ public function __construct(ManagerRegistry $registry) } /** - * Repos reachable from any package-version advisory, with their advisory count. + * Repos reachable from any package-version advisory, with their count of + * advisories not marked as not affected for the repo. * * Path: GitRepo → GitTag → Installation → PackageVersion → Advisory. * @@ -40,6 +43,8 @@ public function findReposWithAdvisoryCount(): array ->innerJoin('gt.installations', 'i') ->innerJoin('i.packageVersions', 'pv') ->innerJoin('pv.advisories', 'a') + ->leftJoin(AdvisoryAssessment::class, 'aa', Join::WITH, 'aa.advisoryId = a.advisoryId AND aa.repoProvider = r.provider AND aa.repoOrganization = r.organization AND aa.repoName = r.repo') + ->andWhere('aa.id IS NULL') ->groupBy('r.id') ->having('COUNT(DISTINCT a.id) > 0') ->orderBy('r.organization', \SortDirection::Ascending) diff --git a/src/Service/AdvisoryAssessor.php b/src/Service/AdvisoryAssessor.php new file mode 100644 index 00000000..e9943b65 --- /dev/null +++ b/src/Service/AdvisoryAssessor.php @@ -0,0 +1,99 @@ +getInstallation()->getGitTag()?->getRepo(); + } + + /** + * The site's advisories, each with its assessment if it is marked. + * + * @return list + */ + public function advisories(Site $site): array + { + $gitRepo = $this->gitRepo($site); + $assessments = null === $gitRepo ? [] : $this->repository->findByGitRepo($gitRepo); + + return array_values(array_map( + static fn (Advisory $advisory): array => ['advisory' => $advisory, 'assessment' => $assessments[$advisory->getAdvisoryId()] ?? null], + $site->getAdvisories()->toArray(), + )); + } + + /** + * Advisories that are not marked as not affected. + */ + public function openCount(Site $site): int + { + return count(array_filter($this->advisories($site), static fn (array $row): bool => null === $row['assessment'])); + } + + /** + * Mark those of the advisories that are open on the site. + * + * @param list $advisoryIds + * + * @return int the number of advisories marked + * + * @throws \InvalidArgumentException when the site has no git repo or the reason is empty + */ + public function markNotAffected(Site $site, array $advisoryIds, string $reason, string $assessedBy): int + { + $gitRepo = $this->gitRepo($site) ?? throw new \InvalidArgumentException('No git repo linked, so advisories cannot be marked as not affected.'); + $reason = trim($reason); + if ('' === $reason) { + throw new \InvalidArgumentException('A reason is required.'); + } + + $marked = 0; + foreach ($this->advisories($site) as ['advisory' => $advisory, 'assessment' => $assessment]) { + if (null !== $assessment || !in_array($advisory->getAdvisoryId(), $advisoryIds, true)) { + continue; + } + $this->entityManager->persist(new AdvisoryAssessment() + ->setGitRepo($gitRepo) + ->setAdvisoryId($advisory->getAdvisoryId()) + ->setReason($reason) + ->setAssessedBy($assessedBy)); + ++$marked; + } + $this->entityManager->flush(); + + return $marked; + } + + public function removeMark(Site $site, string $advisoryId): void + { + $gitRepo = $this->gitRepo($site); + $assessment = null === $gitRepo ? null : ($this->repository->findByGitRepo($gitRepo)[$advisoryId] ?? null); + if (null !== $assessment) { + $this->entityManager->remove($assessment); + $this->entityManager->flush(); + } + } +} diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig new file mode 100644 index 00000000..637bd435 --- /dev/null +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -0,0 +1,78 @@ +{# @var ea \EasyCorp\Bundle\EasyAdminBundle\Context\AdminContext #} +{# @var field \EasyCorp\Bundle\EasyAdminBundle\Dto\FieldDto #} +{# @var entity \EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto #} +{% if ea().crud.currentAction == 'detail' %} + {% set git_repo = field.formattedValue.gitRepo %} + {% set advisories = field.formattedValue.advisories %} + {% set open = advisories|filter(row => row.assessment is null) %} + {% set site_id = entity.instance.id %} + {% set token = csrf_token('site_advisories_' ~ site_id) %} +
+ {% if git_repo %} +

Marks apply to all checkouts of {{ git_repo }}.

+ {% else %} +
No git repo linked, so advisories cannot be marked as not affected.
+ {% endif %} + {% if advisories|length > 0 %} + + + + + + + + + + + {% for row in advisories %} + {% set advisory = row.advisory %} + {% set assessment = row.assessment %} + {% set url = ea_url() + .unsetAll() + .setController('App\\Controller\\Admin\\AdvisoryCrudController') + .setAction('detail') + .setEntityId(advisory.id) %} + + + + + + + {% endfor %} + +
PackageCVETitleStatus
{{ advisory.package }}{{ advisory.cve ?? advisory.advisoryId }}{{ advisory.title }} + {% if assessment %} + Not affected + {{ assessment.reason }} + ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }}) + {% if git_repo %} +
+ + + +
+ {% endif %} + {% elseif git_repo %} +
+ + + + +
+ {% endif %} +
+ {% endif %} + {% if git_repo and open|length > 0 %} +
+ + {% for row in open %} + + {% endfor %} + + +
+ {% endif %} +
+{% elseif field.formattedValue != 0 %} + {{ field.formattedValue }} +{% endif %} diff --git a/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php b/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php new file mode 100644 index 00000000..d44cd6a4 --- /dev/null +++ b/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php @@ -0,0 +1,207 @@ +loggedInClient(); + + $crawler = $client->request('GET', $this->detailUrl('shop.example.com')); + + $this->assertResponseIsSuccessful(); + $this->assertSelectorTextContains('.advisory-assessments', 'Marks apply to all checkouts of example-org/shop'); + $this->assertCount(2, $crawler->filter('form.advisory-mark'), 'One per open advisory'); + $this->assertSelectorExists('form.advisory-accept-all'); + } + + public function testMarkingAnAdvisoryAsNotAffected(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('shop.example.com')); + + $client->submit($this->markForm($crawler, self::TWIG), ['reason' => 'Templates are not user editable']); + + $this->assertResponseRedirects($this->detailUrl('shop.example.com')); + $crawler = $client->followRedirect(); + $this->assertSelectorTextContains('#flash-messages', 'Marked 1 advisory as not affected for all checkouts of example-org/shop'); + $row = $this->row($crawler, self::TWIG); + $this->assertStringContainsString('Not affected', $row->text()); + $this->assertStringContainsString('Templates are not user editable', $row->text()); + $this->assertStringContainsString('admin@example.com', $row->text()); + $this->assertCount(1, $row->filter('form.advisory-remove')); + + $assessment = $this->entityManager()->getRepository(AdvisoryAssessment::class)->findOneBy(['advisoryId' => self::TWIG]); + $this->assertSame('admin@example.com', $assessment?->getAssessedBy()); + } + + public function testAcceptingAllCurrentAdvisories(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('shop.example.com')); + + $client->submit($crawler->filter('form.advisory-accept-all')->form(), ['reason' => 'Reviewed with the client']); + + $crawler = $client->followRedirect(); + $this->assertSelectorTextContains('#flash-messages', 'Marked 2 advisories as not affected'); + $this->assertCount(0, $crawler->filter('form.advisory-mark')); + $this->assertCount(0, $crawler->filter('form.advisory-accept-all'), 'Nothing left to accept'); + } + + public function testAReasonIsRequired(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('shop.example.com')); + + $client->submit($this->markForm($crawler, self::TWIG), ['reason' => ' ']); + + $client->followRedirect(); + $this->assertSelectorTextContains('#flash-messages', 'A reason is required'); + $this->assertSame(0, $this->entityManager()->getRepository(AdvisoryAssessment::class)->count(['advisoryId' => self::TWIG])); + } + + public function testRemovingAMark(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('intranet.example.com')); + + $client->submit($this->row($crawler, 'PKSA-m3r8-5tyv-1h6p')->filter('form.advisory-remove')->form()); + + $crawler = $client->followRedirect(); + $this->assertSelectorTextContains('#flash-messages', 'Removed the mark'); + $this->assertCount(1, $this->row($crawler, 'PKSA-m3r8-5tyv-1h6p')->filter('form.advisory-mark')); + } + + public function testAnInvalidTokenIsRejected(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('shop.example.com')); + $form = $this->markForm($crawler, self::TWIG); + + $client->request('POST', $form->getUri(), ['advisoryIds' => [self::TWIG], 'reason' => 'Reviewed', '_token' => 'invalid']); + + $this->assertResponseStatusCodeSame(403); + } + + public function testASiteWithoutAGitRepoExplainsWhyItCannotBeMarked(): void + { + $client = $this->loggedInClient(); + + $crawler = $client->request('GET', $this->detailUrl('release.example.com')); + + $this->assertResponseIsSuccessful(); + $this->assertSelectorTextContains('.advisory-assessments', 'No git repo linked'); + $this->assertCount(0, $crawler->filter('form.advisory-mark, form.advisory-accept-all')); + $this->assertGreaterThan(0, $crawler->filter('.advisory-assessments tbody tr')->count(), 'The advisories are still listed'); + } + + public function testTheIndexCountsOnlyOpenAdvisories(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('intranet.example.com')); + $this->assertSame('3', $this->indexBadge($client, 'intranet.example.com'), 'Four advisories, one marked in the fixtures'); + + // With all four marked, intranet has the most advisories but none open. + $client->submit($crawler->filter('form.advisory-accept-all')->form(), ['reason' => 'Reviewed']); + + $this->assertSame('', $this->indexBadge($client, 'intranet.example.com')); + $url = static::getContainer()->get(AdminUrlGenerator::class) + ->setController(SiteCrudController::class) + ->setAction(Crud::PAGE_INDEX) + ->set('sort', ['advisoryCount' => 'DESC']) + ->generateUrl(); + + $crawler = $client->request('GET', $url); + + $this->assertResponseIsSuccessful(); + $counts = $crawler->filter('tbody tr[data-id]')->each(static fn (Crawler $row): int => (int) ($row->filter(self::BADGE)->count() > 0 ? $row->filter(self::BADGE)->text() : 0)); + $sorted = $counts; + rsort($sorted); + $this->assertSame($sorted, $counts, 'Sorted by the open count'); + } + + public function testTheRepoAdvisoriesPageCountsOnlyOpenAdvisories(): void + { + $client = $this->loggedInClient(); + + $crawler = $client->request('GET', static::getContainer()->get('router')->generate('admin_repo_advisories')); + + $this->assertResponseIsSuccessful(); + $row = $crawler->filter('tr')->reduce(static fn (Crawler $row): bool => str_contains($row->text(), 'example-org/intranet')); + $this->assertSame('3', $row->filter('.badge-danger')->text()); + } + + private function loggedInClient(): KernelBrowser + { + $client = static::createClient(); + // Keep the rows written by one request for the next. + $client->disableReboot(); + $client->loginUser($this->entityManager()->getRepository(User::class)->findOneBy(['email' => 'admin@example.com'])); + + return $client; + } + + private function entityManager(): EntityManagerInterface + { + return static::getContainer()->get(EntityManagerInterface::class); + } + + private function detailUrl(string $primaryDomain): string + { + $site = $this->entityManager()->getRepository(Site::class)->findOneBy(['primaryDomain' => $primaryDomain]); + + return static::getContainer()->get(AdminUrlGenerator::class) + ->setController(SiteCrudController::class) + ->setAction(Crud::PAGE_DETAIL) + ->setEntityId($site?->getId()) + ->generateUrl(); + } + + private function row(Crawler $crawler, string $advisoryId): Crawler + { + return $crawler->filter('.advisory-assessments tbody tr[data-advisory-id="'.$advisoryId.'"]'); + } + + private function markForm(Crawler $crawler, string $advisoryId): Form + { + return $this->row($crawler, $advisoryId)->filter('form.advisory-mark')->form(); + } + + /** + * The site's advisory badge on the index page, or '' without one. + */ + private function indexBadge(KernelBrowser $client, string $primaryDomain): string + { + $url = static::getContainer()->get(AdminUrlGenerator::class) + ->setController(SiteCrudController::class) + ->setAction(Crud::PAGE_INDEX) + ->generateUrl(); + $badge = $client->request('GET', $url)->filter('tbody tr[data-id]') + ->reduce(static fn (Crawler $row): bool => str_contains($row->text(), $primaryDomain)) + ->filter(self::BADGE); + + return $badge->count() > 0 ? $badge->text() : ''; + } +} diff --git a/tests/DataFixtures/FixturesTest.php b/tests/DataFixtures/FixturesTest.php index 99b7f635..b2991513 100644 --- a/tests/DataFixtures/FixturesTest.php +++ b/tests/DataFixtures/FixturesTest.php @@ -5,6 +5,7 @@ namespace App\Tests\DataFixtures; use App\Entity\Advisory; +use App\Entity\AdvisoryAssessment; use App\Entity\CodeOwner; use App\Entity\DetectionResult; use App\Entity\DockerImage; @@ -199,6 +200,7 @@ public function testFixturesUseExampleDataOnly(): void ...array_map(static fn (User $user): string => (string) $user->getEmail(), $this->entityManager->getRepository(User::class)->findAll()), ...array_map(static fn (CodeOwner $owner): string => $owner->getEmail(), $this->entityManager->getRepository(CodeOwner::class)->findAll()), ...array_filter(array_map(static fn (SecurityContract $contract): ?string => $contract->getClientContactEmail(), $this->entityManager->getRepository(SecurityContract::class)->findAll())), + ...array_map(static fn (AdvisoryAssessment $assessment): string => $assessment->getAssessedBy(), $this->entityManager->getRepository(AdvisoryAssessment::class)->findAll()), ]; foreach ($emails as $email) { $this->assertMatchesRegularExpression('/@example\.com$/', $email); diff --git a/tests/Service/AdvisoryAssessorTest.php b/tests/Service/AdvisoryAssessorTest.php new file mode 100644 index 00000000..6aad741d --- /dev/null +++ b/tests/Service/AdvisoryAssessorTest.php @@ -0,0 +1,113 @@ +entityManager = self::getContainer()->get(EntityManagerInterface::class); + $this->assessor = self::getContainer()->get(AdvisoryAssessor::class); + } + + public function testAMarkAppliesToEveryCheckoutOfTheRepo(): void + { + $this->assertSame(2, $this->assessor->openCount($this->site('shop.example.com'))); + $this->assertSame(1, $this->assessor->openCount($this->site('stg.shop.example.com'))); + + $this->assessor->markNotAffected($this->site('shop.example.com'), [self::TWIG], 'Templates are not user editable', 'user@example.com'); + + // A new advisory, here http-foundation, still counts. + $this->assertSame(1, $this->assessor->openCount($this->site('shop.example.com'))); + $this->assertSame(0, $this->assessor->openCount($this->site('stg.shop.example.com'))); + } + + public function testOnlyTheSitesOpenAdvisoriesAreMarked(): void + { + $marked = $this->assessor->markNotAffected($this->site('stg.shop.example.com'), [self::TWIG, self::HTTP_FOUNDATION], 'Reviewed', 'user@example.com'); + + $this->assertSame(1, $marked); + $this->assertSame(1, $this->assessor->openCount($this->site('shop.example.com')), 'http-foundation is not on stg'); + } + + public function testAMarkRecordsTheReasonAndWhoMadeIt(): void + { + $this->assessor->markNotAffected($this->site('shop.example.com'), [self::TWIG], ' Templates are not user editable ', 'user@example.com'); + + $assessment = $this->entityManager->getRepository(AdvisoryAssessment::class)->findOneBy(['advisoryId' => self::TWIG]); + $this->assertSame(['github.com', 'example-org', 'shop'], [$assessment->getRepoProvider(), $assessment->getRepoOrganization(), $assessment->getRepoName()]); + $this->assertSame('Templates are not user editable', $assessment->getReason()); + $this->assertSame('user@example.com', $assessment->getAssessedBy()); + } + + public function testAReasonIsRequired(): void + { + $this->expectException(\InvalidArgumentException::class); + + $this->assessor->markNotAffected($this->site('shop.example.com'), [self::TWIG], ' ', 'user@example.com'); + } + + public function testASiteWithoutAGitRepoCannotBeMarked(): void + { + $site = $this->site('release.example.com'); + $this->assertNull($this->assessor->gitRepo($site)); + + $this->expectException(\InvalidArgumentException::class); + + $this->assessor->markNotAffected($site, ['SA-CORE-2099-001'], 'Reviewed', 'user@example.com'); + } + + public function testRemovingAMarkCountsTheAdvisoryAgain(): void + { + $site = $this->site('intranet.example.com'); + $this->assertSame(3, $this->assessor->openCount($site), 'The fixtures mark commonmark'); + + $this->assessor->removeMark($site, 'PKSA-m3r8-5tyv-1h6p'); + + $this->assertSame(4, $this->assessor->openCount($site)); + } + + public function testAdvisoriesListTheirMark(): void + { + $marks = []; + foreach ($this->assessor->advisories($this->site('intranet.example.com')) as ['advisory' => $advisory, 'assessment' => $assessment]) { + $marks[$advisory->getAdvisoryId()] = $assessment?->getReason(); + } + ksort($marks); + + $this->assertSame([ + 'PKSA-d8k2-6vnp-4j9t' => null, + 'PKSA-m3r8-5tyv-1h6p' => 'Markdown is only rendered from editor input', + 'SA-CONTRIB-2099-001' => null, + 'SA-CORE-2099-001' => null, + ], $marks); + } + + private function site(string $primaryDomain): Site + { + return $this->entityManager->getRepository(Site::class)->findOneBy(['primaryDomain' => $primaryDomain]); + } +} From 30c41d1cdddebbef9ccfe7ffe6cee09aa9c12e5b Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 17:41:19 +0200 Subject: [PATCH 02/15] feat: ask for the not affected reason in a modal --- .../Fields/_advisory_reason_modal.html.twig | 15 +++++++++++++++ .../Fields/site_advisories.html.twig | 10 ++++------ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig diff --git a/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig b/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig new file mode 100644 index 00000000..f20c4f75 --- /dev/null +++ b/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig @@ -0,0 +1,15 @@ +{# A button opening a modal that asks for the reason and submits the enclosing form. #} +{{ label|default('Mark') }} + + + {# Blocks do not see the include's variables, so the label wraps the field instead of naming its id. #} + + + + Cancel + Mark as not affected + + diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 637bd435..ed5b0ad2 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -49,11 +49,10 @@ {% endif %} {% elseif git_repo %} -
+ - - + {{ include('EasyAdminBundle/Fields/_advisory_reason_modal.html.twig', {id: 'advisory-mark-' ~ loop.index, title: 'Mark ' ~ (advisory.cve ?? advisory.advisoryId) ~ ' as not affected?'}) }}
{% endif %} @@ -63,13 +62,12 @@ {% endif %} {% if git_repo and open|length > 0 %} -
+ {% for row in open %} {% endfor %} - - + {{ include('EasyAdminBundle/Fields/_advisory_reason_modal.html.twig', {id: 'advisory-accept-all', title: 'Mark all ' ~ open|length ~ ' open advisories as not affected?', label: 'Mark all'}) }}
{% endif %} From 9daac1ce8d4d66d54e2231d868245f9fbbccb966 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 17:47:15 +0200 Subject: [PATCH 03/15] fix: submit advisory marks once and report repeat marks --- assets/admin.js | 10 ++++++++++ .../Admin/AdvisoryAssessmentController.php | 16 ++++++++++------ .../Fields/site_advisories.html.twig | 6 +++--- .../Admin/AdvisoryAssessmentControllerTest.php | 14 ++++++++++++++ 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/assets/admin.js b/assets/admin.js index 7bd14d4e..5f1ac6c2 100644 --- a/assets/admin.js +++ b/assets/admin.js @@ -21,6 +21,16 @@ const tomSelectHandler = () => { window.addEventListener("DOMContentLoaded", tomSelectHandler); document.addEventListener("ea.collection.item-added", tomSelectHandler); +// Disable the buttons of a [data-submit-once] form once it submits, so a +// slow page load does not invite a second click. +document.addEventListener("submit", (event) => { + if (event.target.matches("[data-submit-once]")) { + event.target + .querySelectorAll("button") + .forEach((button) => (button.disabled = true)); + } +}); + // Copy the text of a [data-copy-text] button to the clipboard, and show a // check mark for a moment, like GitHub's copy buttons. document.addEventListener("click", async (event) => { diff --git a/src/Controller/Admin/AdvisoryAssessmentController.php b/src/Controller/Admin/AdvisoryAssessmentController.php index 3d6870d3..01f2ca77 100644 --- a/src/Controller/Admin/AdvisoryAssessmentController.php +++ b/src/Controller/Admin/AdvisoryAssessmentController.php @@ -40,12 +40,16 @@ public function markNotAffected(#[MapEntity(id: 'id')] Site $site, Request $requ $request->request->getString('reason'), (string) $this->getUser()?->getUserIdentifier(), ); - $this->addFlash('success', sprintf( - 'Marked %d %s as not affected for all checkouts of %s.', - $marked, - 1 === $marked ? 'advisory' : 'advisories', - $this->assessor->gitRepo($site), - )); + if (0 === $marked) { + $this->addFlash('info', 'These advisories are already marked as not affected.'); + } else { + $this->addFlash('success', sprintf( + 'Marked %d %s as not affected for all checkouts of %s.', + $marked, + 1 === $marked ? 'advisory' : 'advisories', + $this->assessor->gitRepo($site), + )); + } } catch (\InvalidArgumentException $exception) { $this->addFlash('danger', $exception->getMessage()); } diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index ed5b0ad2..09de2456 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -42,14 +42,14 @@ {{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }}) {% if git_repo %} -
+
{% endif %} {% elseif git_repo %} -
+ {{ include('EasyAdminBundle/Fields/_advisory_reason_modal.html.twig', {id: 'advisory-mark-' ~ loop.index, title: 'Mark ' ~ (advisory.cve ?? advisory.advisoryId) ~ ' as not affected?'}) }} @@ -62,7 +62,7 @@ {% endif %} {% if git_repo and open|length > 0 %} - + {% for row in open %} diff --git a/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php b/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php index d44cd6a4..ec7185ee 100644 --- a/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php +++ b/tests/Controller/Admin/AdvisoryAssessmentControllerTest.php @@ -57,6 +57,20 @@ public function testMarkingAnAdvisoryAsNotAffected(): void $this->assertSame('admin@example.com', $assessment?->getAssessedBy()); } + public function testResubmittingAMarkSaysItIsAlreadyMarked(): void + { + $client = $this->loggedInClient(); + $crawler = $client->request('GET', $this->detailUrl('shop.example.com')); + $form = $this->markForm($crawler, self::TWIG); + $client->submit($form, ['reason' => 'Reviewed']); + + $client->submit($form, ['reason' => 'Reviewed']); + + $client->followRedirect(); + $this->assertSelectorTextContains('#flash-messages', 'already marked as not affected'); + $this->assertSelectorTextNotContains('#flash-messages', 'Marked 0'); + } + public function testAcceptingAllCurrentAdvisories(): void { $client = $this->loggedInClient(); From 7ac2d7c484e9ecc0cd763eeb92c5f2e0522f19c7 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 18:42:43 +0200 Subject: [PATCH 04/15] style: show the missing git repo as a badge --- templates/EasyAdminBundle/Fields/site_advisories.html.twig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 09de2456..42f7b405 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -11,7 +11,7 @@ {% if git_repo %}

Marks apply to all checkouts of {{ git_repo }}.

{% else %} -
No git repo linked, so advisories cannot be marked as not affected.
+

No git repo linked Advisories cannot be marked as not affected.

{% endif %} {% if advisories|length > 0 %} From e70fcaa60e1a37d1b7fea5ade7935ea4d8819195 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 18:56:09 +0200 Subject: [PATCH 05/15] style: move the mark details under the advisory title --- .../Fields/site_advisories.html.twig | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 42f7b405..2d54fc08 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -34,18 +34,21 @@ .setEntityId(advisory.id) %} - - + + - + @@ -38,12 +38,11 @@ + - - - + + +
{{ advisory.package }}{{ advisory.cve ?? advisory.advisoryId }}{{ advisory.title }}{{ advisory.cve ?? advisory.advisoryId }} + {{ advisory.title }} + {% if assessment %} +
{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
+ {% endif %} + 		{% if assessment %} Not affected - {{ assessment.reason }} - ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }}) {% if git_repo %} - + Unmark {% endif %} {% elseif git_repo %} From dc1502e9843a0c2b0682d13c752067ecaac5eadd Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 18:57:55 +0200 Subject: [PATCH 06/15] style: put the not affected pill on the mark details line --- templates/EasyAdminBundle/Fields/site_advisories.html.twig | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 2d54fc08..40f24f5f 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -20,7 +20,7 @@ Package CVE TitleStatus
{{ advisory.title }} {% if assessment %} -
{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
+
Not affected {{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
{% endif %} 		{% if assessment %} - Not affected {% if git_repo %}
From d9a8428215a8cf107bd52e79a037b2ac3205d07f Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 19:07:18 +0200 Subject: [PATCH 07/15] style: space out the mark details line --- templates/EasyAdminBundle/Fields/site_advisories.html.twig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 40f24f5f..2b0fa4df 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -38,7 +38,7 @@
{{ advisory.title }} {% if assessment %} -
Not affected {{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
+
Not affected{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
{% endif %} 		From 1776586457bc1c579e78235e6723fb62c50cb67f Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 19:08:06 +0200 Subject: [PATCH 08/15] style: fade the mark details line --- templates/EasyAdminBundle/Fields/site_advisories.html.twig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 2b0fa4df..38ff7393 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -38,7 +38,7 @@ {{ advisory.title }} {% if assessment %} -
Not affected{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
+
Not affected{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
{% endif %} 		From 34517cf38e2cfcfe2c7c9726087085579fa6db30 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 19:08:59 +0200 Subject: [PATCH 09/15] style: fade the whole row of a marked advisory --- templates/EasyAdminBundle/Fields/site_advisories.html.twig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 38ff7393..753823d3 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -32,13 +32,13 @@ .setController('App\\Controller\\Admin\\AdvisoryCrudController') .setAction('detail') .setEntityId(advisory.id) %} -
{{ advisory.package }} {{ advisory.cve ?? advisory.advisoryId }} {{ advisory.title }} {% if assessment %} -
Not affected{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
+
Not affected{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
{% endif %} 		From 8fbcce81f27e5b38460e240f811e0e43c071aaca Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 19:09:36 +0200 Subject: [PATCH 10/15] style: keep the unmark button out of the fade --- .../EasyAdminBundle/Fields/site_advisories.html.twig | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/templates/EasyAdminBundle/Fields/site_advisories.html.twig b/templates/EasyAdminBundle/Fields/site_advisories.html.twig index 753823d3..d6b8cdb7 100644 --- a/templates/EasyAdminBundle/Fields/site_advisories.html.twig +++ b/templates/EasyAdminBundle/Fields/site_advisories.html.twig @@ -32,10 +32,12 @@ .setController('App\\Controller\\Admin\\AdvisoryCrudController') .setAction('detail') .setEntityId(advisory.id) %} -
{{ advisory.package }}{{ advisory.cve ?? advisory.advisoryId }} + {# Fades the cells, not the row, so the Unmark button stays at full strength. #} + {% set faded = assessment ? 'text-body-secondary opacity-50' %} +
{{ advisory.package }}{{ advisory.cve ?? advisory.advisoryId }} {{ advisory.title }} {% if assessment %}
Not affected{{ assessment.reason }} ({{ assessment.assessedBy }}, {{ assessment.createdAt|date('Y-m-d') }})
From 0bb2405f4d4e3d2af7729221654b47fc469d1fe9 Mon Sep 17 00:00:00 2001 From: turegjorup Date: Wed, 21 Oct 2026 19:20:16 +0200 Subject: [PATCH 11/15] style: space the reason field from the modal description --- .../EasyAdminBundle/Fields/_advisory_reason_modal.html.twig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig b/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig index f20c4f75..7437ead2 100644 --- a/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig +++ b/templates/EasyAdminBundle/Fields/_advisory_reason_modal.html.twig @@ -3,7 +3,7 @@ {# Blocks do not see the include's variables, so the label wraps the field instead of naming its id. #} -