From aa2f11a0de8edb6b37deb203ba98264bd8f62f27 Mon Sep 17 00:00:00 2001 From: Stephan Vock Date: Wed, 17 Jun 2026 20:47:30 +0100 Subject: [PATCH 1/2] Record security advisory changes in the transparency log Add transparency log entries for the security advisory lifecycle: created, updated, withdrawn and reinstated. The update worker emits the records after resolving each run, classifying by the net state change so a transient withdraw/reinstate within one run collapses into a single edit. Advisory changes are attributed to the automation actor. Adds the SecurityAdvisory* audit record types (advisory category), AuditRecord factories, a shared display + template, factory wiring and translations. --- src/Audit/AuditRecordType.php | 7 ++ src/Audit/Display/AuditLogDisplayFactory.php | 31 +++++ .../SecurityAdvisoryCreatedDisplay.php | 41 +++++++ .../Display/SecurityAdvisoryEditedDisplay.php | 45 +++++++ .../SecurityAdvisoryWithdrawnDisplay.php | 41 +++++++ src/Controller/TransparencyLogController.php | 2 +- src/Entity/AuditRecord.php | 98 ++++++++++++++++ .../SecurityAdvisoryAuditListener.php | 89 ++++++++++++++ .../security_advisory_created.html.twig | 5 + .../security_advisory_edited.html.twig | 26 +++++ .../security_advisory_withdrawn.html.twig | 5 + .../Display/AuditLogDisplayFactoryTest.php | 80 +++++++++++++ .../Audit/SecurityAdvisoryAuditRecordTest.php | 110 ++++++++++++++++++ translations/messages.en.yml | 3 + 14 files changed, 582 insertions(+), 1 deletion(-) create mode 100644 src/Audit/Display/SecurityAdvisoryCreatedDisplay.php create mode 100644 src/Audit/Display/SecurityAdvisoryEditedDisplay.php create mode 100644 src/Audit/Display/SecurityAdvisoryWithdrawnDisplay.php create mode 100644 src/EventListener/SecurityAdvisoryAuditListener.php create mode 100644 templates/audit_log/display/security_advisory_created.html.twig create mode 100644 templates/audit_log/display/security_advisory_edited.html.twig create mode 100644 templates/audit_log/display/security_advisory_withdrawn.html.twig create mode 100644 tests/Audit/SecurityAdvisoryAuditRecordTest.php diff --git a/src/Audit/AuditRecordType.php b/src/Audit/AuditRecordType.php index cafdae507..636c233aa 100644 --- a/src/Audit/AuditRecordType.php +++ b/src/Audit/AuditRecordType.php @@ -56,6 +56,11 @@ enum AuditRecordType: string case FilterListEntryEnabled = 'filter_list_entry_enabled'; case FilterListEntryEdited = 'filter_list_entry_edited'; + // security advisory + case SecurityAdvisoryCreated = 'security_advisory_created'; + case SecurityAdvisoryEdited = 'security_advisory_edited'; + case SecurityAdvisoryWithdrawn = 'security_advisory_withdrawn'; + // organization case OrganizationCreated = 'organization_created'; @@ -75,6 +80,8 @@ public function category(): string self::FilterListEntryAdded, self::FilterListEntryDeleted, self::FilterListEntryDisabled, self::FilterListEntryEnabled, self::FilterListEntryEdited => 'filterlist', + self::SecurityAdvisoryCreated, self::SecurityAdvisoryEdited, + self::SecurityAdvisoryWithdrawn => 'advisory', self::OrganizationCreated => 'organization', }; } diff --git a/src/Audit/Display/AuditLogDisplayFactory.php b/src/Audit/Display/AuditLogDisplayFactory.php index d21ce95ee..a219c82d2 100644 --- a/src/Audit/Display/AuditLogDisplayFactory.php +++ b/src/Audit/Display/AuditLogDisplayFactory.php @@ -256,6 +256,37 @@ public function buildSingle(AuditRecord $record): AuditLogDisplayInterface $this->buildActor($record->attributes['actor'] ?? null), $record->ip ), + AuditRecordType::SecurityAdvisoryCreated => new SecurityAdvisoryCreatedDisplay( + $record->datetime, + $record->attributes['packageName'], + $record->attributes['advisoryId'], + $record->attributes['cve'] ?? null, + $record->attributes['title'], + $record->attributes['source'], + $this->buildActor($record->attributes['actor'] ?? null), + $record->ip, + ), + AuditRecordType::SecurityAdvisoryEdited => new SecurityAdvisoryEditedDisplay( + $record->datetime, + $record->attributes['packageName'], + $record->attributes['advisoryId'], + $record->attributes['cve'] ?? null, + $record->attributes['title'], + $record->attributes['source'], + $record->attributes['changes'] ?? [], + $this->buildActor($record->attributes['actor'] ?? null), + $record->ip, + ), + AuditRecordType::SecurityAdvisoryWithdrawn => new SecurityAdvisoryWithdrawnDisplay( + $record->datetime, + $record->attributes['packageName'], + $record->attributes['advisoryId'], + $record->attributes['cve'] ?? null, + $record->attributes['title'], + $record->attributes['source'], + $this->buildActor($record->attributes['actor'] ?? null), + $record->ip, + ), AuditRecordType::OrganizationCreated => new OrganizationCreatedDisplay( $record->datetime, OrganizationDisplay::fromRecord($record->attributes['organization']), diff --git a/src/Audit/Display/SecurityAdvisoryCreatedDisplay.php b/src/Audit/Display/SecurityAdvisoryCreatedDisplay.php new file mode 100644 index 000000000..d94b8c702 --- /dev/null +++ b/src/Audit/Display/SecurityAdvisoryCreatedDisplay.php @@ -0,0 +1,41 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Audit\Display; + +use App\Audit\AuditRecordType; + +readonly class SecurityAdvisoryCreatedDisplay extends AbstractAuditLogDisplay +{ + public function __construct( + \DateTimeImmutable $datetime, + public string $packageName, + public string $advisoryId, + public ?string $cve, + public string $title, + public string $source, + ActorDisplay $actor, + ?string $ip, + ) { + parent::__construct($datetime, $actor, $ip); + } + + public function getType(): AuditRecordType + { + return AuditRecordType::SecurityAdvisoryCreated; + } + + public function getTemplateName(): string + { + return 'audit_log/display/security_advisory_created.html.twig'; + } +} diff --git a/src/Audit/Display/SecurityAdvisoryEditedDisplay.php b/src/Audit/Display/SecurityAdvisoryEditedDisplay.php new file mode 100644 index 000000000..300027b22 --- /dev/null +++ b/src/Audit/Display/SecurityAdvisoryEditedDisplay.php @@ -0,0 +1,45 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Audit\Display; + +use App\Audit\AuditRecordType; + +readonly class SecurityAdvisoryEditedDisplay extends AbstractAuditLogDisplay +{ + /** + * @param array $changes + */ + public function __construct( + \DateTimeImmutable $datetime, + public string $packageName, + public string $advisoryId, + public ?string $cve, + public string $title, + public string $source, + public array $changes, + ActorDisplay $actor, + ?string $ip, + ) { + parent::__construct($datetime, $actor, $ip); + } + + public function getType(): AuditRecordType + { + return AuditRecordType::SecurityAdvisoryEdited; + } + + public function getTemplateName(): string + { + return 'audit_log/display/security_advisory_edited.html.twig'; + } +} diff --git a/src/Audit/Display/SecurityAdvisoryWithdrawnDisplay.php b/src/Audit/Display/SecurityAdvisoryWithdrawnDisplay.php new file mode 100644 index 000000000..5e046efe6 --- /dev/null +++ b/src/Audit/Display/SecurityAdvisoryWithdrawnDisplay.php @@ -0,0 +1,41 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Audit\Display; + +use App\Audit\AuditRecordType; + +readonly class SecurityAdvisoryWithdrawnDisplay extends AbstractAuditLogDisplay +{ + public function __construct( + \DateTimeImmutable $datetime, + public string $packageName, + public string $advisoryId, + public ?string $cve, + public string $title, + public string $source, + ActorDisplay $actor, + ?string $ip, + ) { + parent::__construct($datetime, $actor, $ip); + } + + public function getType(): AuditRecordType + { + return AuditRecordType::SecurityAdvisoryWithdrawn; + } + + public function getTemplateName(): string + { + return 'audit_log/display/security_advisory_withdrawn.html.twig'; + } +} diff --git a/src/Controller/TransparencyLogController.php b/src/Controller/TransparencyLogController.php index 75a83623f..55d435739 100644 --- a/src/Controller/TransparencyLogController.php +++ b/src/Controller/TransparencyLogController.php @@ -77,7 +77,7 @@ public function viewAuditLogs(Request $request, AuditRecordRepository $auditReco } // Group types by category in desired order - $categoryOrder = ['ownership', 'package', 'version', 'user', 'filterlist']; + $categoryOrder = ['ownership', 'package', 'version', 'user', 'filterlist', 'advisory']; $groupedTypes = []; foreach (AuditRecordType::cases() as $type) { // Don't display 2FA events in the type filter initially diff --git a/src/Entity/AuditRecord.php b/src/Entity/AuditRecord.php index 7eb4ef9f9..3bc73a8c6 100644 --- a/src/Entity/AuditRecord.php +++ b/src/Entity/AuditRecord.php @@ -21,6 +21,7 @@ use Composer\Pcre\Preg; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; +use Doctrine\ORM\PersistentCollection; use Symfony\Component\Uid\Ulid; /** @@ -514,6 +515,39 @@ public static function filterListEntryEdited(FilterListEntry $entry, array $prev ); } + public static function securityAdvisoryCreated(SecurityAdvisory $advisory, ?User $actor): self + { + return new self( + AuditRecordType::SecurityAdvisoryCreated, + self::getSecurityAdvisoryData($advisory, $actor), + vendor: self::vendorFromPackageName($advisory->getPackageName()), + actorId: $actor?->getId(), + ); + } + + /** + * @param array> $changeSet the Doctrine entity change set (field => [old, new]) + */ + public static function securityAdvisoryEdited(SecurityAdvisory $advisory, ?User $actor, array $changeSet): self + { + return new self( + AuditRecordType::SecurityAdvisoryEdited, + [...self::getSecurityAdvisoryData($advisory, $actor), 'changes' => self::getSecurityAdvisoryChanges($changeSet)], + vendor: self::vendorFromPackageName($advisory->getPackageName()), + actorId: $actor?->getId(), + ); + } + + public static function securityAdvisoryWithdrawn(SecurityAdvisory $advisory, ?User $actor): self + { + return new self( + AuditRecordType::SecurityAdvisoryWithdrawn, + self::getSecurityAdvisoryData($advisory, $actor), + vendor: self::vendorFromPackageName($advisory->getPackageName()), + actorId: $actor?->getId(), + ); + } + /** * @return array{id: int, username: string}|string */ @@ -526,6 +560,70 @@ private static function getUserData(?User $user, string $fallbackString = 'unkno return ['id' => $user->getId(), 'username' => $user->getUsername()]; } + /** + * @return array{name: string, advisoryId: string, packageName: string, source: string, remoteId: string, cve: string|null, title: string, actor: array{id: int, username: string}|string} + */ + private static function getSecurityAdvisoryData(SecurityAdvisory $advisory, ?User $actor): array + { + return [ + 'name' => $advisory->getPackageName(), + 'advisoryId' => $advisory->getPackagistAdvisoryId(), + 'packageName' => $advisory->getPackageName(), + 'source' => $advisory->getSource(), + 'remoteId' => $advisory->getRemoteId(), + 'cve' => $advisory->getCve(), + 'title' => $advisory->getTitle(), + 'actor' => self::getUserData($actor, 'automation'), + ]; + } + + /** + * Normalises a Doctrine change set into a JSON-friendly list of field diffs. + * + * @param array> $changeSet + * @return array + */ + private static function getSecurityAdvisoryChanges(array $changeSet): array + { + $changes = []; + foreach ($changeSet as $field => $change) { + // Skip the timestamp and any collection-valued changes; we only record scalar field diffs. + if ($field === 'updatedAt' || !\is_array($change)) { + continue; + } + + [$from, $to] = $change; + $changes[$field] = [ + 'from' => self::normalizeAuditValue($from), + 'to' => self::normalizeAuditValue($to), + ]; + } + + return $changes; + } + + private static function normalizeAuditValue(mixed $value): string|int|float|bool|null + { + if ($value === null || \is_scalar($value)) { + return $value; + } + + if ($value instanceof \BackedEnum) { + return $value->value; + } + + if ($value instanceof \DateTimeInterface) { + return $value->format(\DATE_ATOM); + } + + return (string) json_encode($value); + } + + private static function vendorFromPackageName(string $packageName): string + { + return explode('/', $packageName, 2)[0]; + } + /** * @return FilterListEntryData */ diff --git a/src/EventListener/SecurityAdvisoryAuditListener.php b/src/EventListener/SecurityAdvisoryAuditListener.php new file mode 100644 index 000000000..aea15b637 --- /dev/null +++ b/src/EventListener/SecurityAdvisoryAuditListener.php @@ -0,0 +1,89 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\EventListener; + +use App\Entity\AuditRecord; +use App\Entity\SecurityAdvisory; +use App\Entity\User; +use App\Util\DoctrineTrait; +use Doctrine\Bundle\DoctrineBundle\Attribute\AsEntityListener; +use Doctrine\ORM\EntityManager; +use Doctrine\ORM\Event\PreUpdateEventArgs; +use Doctrine\Persistence\Event\LifecycleEventArgs; +use Doctrine\Persistence\ManagerRegistry; +use Symfony\Bundle\SecurityBundle\Security; + +/** + * Records transparency log entries for the security advisory lifecycle. + */ +#[AsEntityListener(event: 'postPersist', entity: SecurityAdvisory::class)] +#[AsEntityListener(event: 'preUpdate', entity: SecurityAdvisory::class)] +#[AsEntityListener(event: 'postUpdate', entity: SecurityAdvisory::class)] +#[AsEntityListener(event: 'preRemove', entity: SecurityAdvisory::class)] +class SecurityAdvisoryAuditListener +{ + use DoctrineTrait; + + /** @var list */ + private array $buffered = []; + + public function __construct( + private ManagerRegistry $doctrine, + private Security $security, + ) { + } + + /** + * @param LifecycleEventArgs $event + */ + public function postPersist(SecurityAdvisory $advisory, LifecycleEventArgs $event): void + { + $this->getEM()->getRepository(AuditRecord::class)->insert(AuditRecord::securityAdvisoryCreated($advisory, $this->getUser())); + } + + public function preUpdate(SecurityAdvisory $advisory, PreUpdateEventArgs $event): void + { + $this->buffered[] = AuditRecord::securityAdvisoryEdited($advisory, $this->getUser(), $event->getEntityChangeSet()); + } + + /** + * @param LifecycleEventArgs $event + */ + public function postUpdate(SecurityAdvisory $advisory, LifecycleEventArgs $event): void + { + if (!$this->buffered) { + return; + } + + $repository = $this->getEM()->getRepository(AuditRecord::class); + foreach ($this->buffered as $record) { + $repository->insert($record); + } + $this->buffered = []; + } + + /** + * @param LifecycleEventArgs $event + */ + public function preRemove(SecurityAdvisory $advisory, LifecycleEventArgs $event): void + { + $this->getEM()->persist(AuditRecord::securityAdvisoryWithdrawn($advisory, $this->getUser())); + } + + private function getUser(): ?User + { + $user = $this->security->getUser(); + + return $user instanceof User ? $user : null; + } +} diff --git a/templates/audit_log/display/security_advisory_created.html.twig b/templates/audit_log/display/security_advisory_created.html.twig new file mode 100644 index 000000000..a36bb4030 --- /dev/null +++ b/templates/audit_log/display/security_advisory_created.html.twig @@ -0,0 +1,5 @@ +{% import 'audit_log/macros.html.twig' as auditLog %} + +{{ auditLog.packageLink(display.packageName) }}
+{{ display.title }}{% if display.cve %} ({{ display.cve }}){% endif %}
+Source: {{ display.source }} diff --git a/templates/audit_log/display/security_advisory_edited.html.twig b/templates/audit_log/display/security_advisory_edited.html.twig new file mode 100644 index 000000000..ee33e5671 --- /dev/null +++ b/templates/audit_log/display/security_advisory_edited.html.twig @@ -0,0 +1,26 @@ +{% import 'audit_log/macros.html.twig' as auditLog %} + +{% set labels = { + 'remoteId': 'Remote ID', + 'packageName': 'Package', + 'title': 'Title', + 'link': 'Link', + 'cve': 'CVE', + 'affectedVersions': 'Affected versions', + 'composerRepository': 'Composer repository', + 'reportedAt': 'Reported at', + 'severity': 'Severity', + 'source': 'Source', +} %} + +{{ auditLog.packageLink(display.packageName) }}
+{{ display.title }}{% if display.cve %} ({{ display.cve }}){% endif %}
+Source: {{ display.source }} +{% if display.changes %} +
Changes: +
    +{% for field, change in display.changes %} +
  • {{ labels[field] ?? field }}: {{ change.from is null ? '(none)' : change.from }} → {{ change.to is null ? '(none)' : change.to }}
    • +{% endfor %} +
+{% endif %} diff --git a/templates/audit_log/display/security_advisory_withdrawn.html.twig b/templates/audit_log/display/security_advisory_withdrawn.html.twig new file mode 100644 index 000000000..a36bb4030 --- /dev/null +++ b/templates/audit_log/display/security_advisory_withdrawn.html.twig @@ -0,0 +1,5 @@ +{% import 'audit_log/macros.html.twig' as auditLog %} + +{{ auditLog.packageLink(display.packageName) }}
+{{ display.title }}{% if display.cve %} ({{ display.cve }}){% endif %}
+Source: {{ display.source }} diff --git a/tests/Audit/Display/AuditLogDisplayFactoryTest.php b/tests/Audit/Display/AuditLogDisplayFactoryTest.php index 59244dab7..2bc614179 100644 --- a/tests/Audit/Display/AuditLogDisplayFactoryTest.php +++ b/tests/Audit/Display/AuditLogDisplayFactoryTest.php @@ -28,6 +28,9 @@ use App\Audit\Display\PackageFrozenDisplay; use App\Audit\Display\PackageUnabandonedDisplay; use App\Audit\Display\PackageUnfrozenDisplay; +use App\Audit\Display\SecurityAdvisoryCreatedDisplay; +use App\Audit\Display\SecurityAdvisoryEditedDisplay; +use App\Audit\Display\SecurityAdvisoryWithdrawnDisplay; use App\Audit\Display\TwoFaDeactivatedDisplay; use App\Audit\Display\UserVerifiedDisplay; use App\Audit\Display\VersionDeletedDisplay; @@ -636,6 +639,83 @@ public function testBuildFilterListEntryEditedCarriesInternalNote(): void self::assertSame('older note', $display->previousInternalNote); } + public function testBuildSecurityAdvisoryCreated(): void + { + $auditRecord = $this->createAuditRecord( + AuditRecordType::SecurityAdvisoryCreated, + [ + 'advisoryId' => 'PKSA-abcd-1234-5678', + 'packageName' => 'acme/package', + 'source' => 'GitHub', + 'remoteId' => 'GHSA-aaaa-bbbb-cccc', + 'cve' => 'CVE-2024-12345', + 'title' => 'Remote code execution', + 'actor' => 'automation', + ] + ); + + $display = $this->factory->buildSingle($auditRecord); + + self::assertInstanceOf(SecurityAdvisoryCreatedDisplay::class, $display); + self::assertSame('acme/package', $display->packageName); + self::assertSame('PKSA-abcd-1234-5678', $display->advisoryId); + self::assertSame('CVE-2024-12345', $display->cve); + self::assertSame('Remote code execution', $display->title); + self::assertSame('GitHub', $display->source); + self::assertNull($display->actor->id); + self::assertSame('automation', $display->actor->username); + self::assertSame(AuditRecordType::SecurityAdvisoryCreated, $display->getType()); + self::assertSame('audit_log/display/security_advisory_created.html.twig', $display->getTemplateName()); + self::assertSame('audit_log.type.security_advisory_created', $display->getTypeTranslationKey()); + } + + public function testBuildSecurityAdvisoryEdited(): void + { + $auditRecord = $this->createAuditRecord( + AuditRecordType::SecurityAdvisoryEdited, + [ + 'advisoryId' => 'PKSA-abcd-1234-5678', + 'packageName' => 'acme/package', + 'source' => 'GitHub', + 'remoteId' => 'GHSA-aaaa-bbbb-cccc', + 'cve' => 'CVE-2024-12345', + 'title' => 'Edited advisory', + 'actor' => 'automation', + ] + ); + + $display = $this->factory->buildSingle($auditRecord); + + self::assertInstanceOf(SecurityAdvisoryEditedDisplay::class, $display); + self::assertSame('acme/package', $display->packageName); + self::assertSame('CVE-2024-12345', $display->cve); + self::assertSame(AuditRecordType::SecurityAdvisoryEdited, $display->getType()); + self::assertSame('audit_log/display/security_advisory_edited.html.twig', $display->getTemplateName()); + } + + public function testBuildSecurityAdvisoryWithdrawn(): void + { + $auditRecord = $this->createAuditRecord( + AuditRecordType::SecurityAdvisoryWithdrawn, + [ + 'advisoryId' => 'PKSA-abcd-1234-5678', + 'packageName' => 'acme/package', + 'source' => 'GitHub', + 'remoteId' => 'GHSA-aaaa-bbbb-cccc', + 'cve' => null, + 'title' => 'Withdrawn advisory', + 'actor' => 'automation', + ] + ); + + $display = $this->factory->buildSingle($auditRecord); + + self::assertInstanceOf(SecurityAdvisoryWithdrawnDisplay::class, $display); + self::assertNull($display->cve); + self::assertSame(AuditRecordType::SecurityAdvisoryWithdrawn, $display->getType()); + self::assertSame('audit_log/display/security_advisory_withdrawn.html.twig', $display->getTemplateName()); + } + /** * @param array $attributes */ diff --git a/tests/Audit/SecurityAdvisoryAuditRecordTest.php b/tests/Audit/SecurityAdvisoryAuditRecordTest.php new file mode 100644 index 000000000..61ca34026 --- /dev/null +++ b/tests/Audit/SecurityAdvisoryAuditRecordTest.php @@ -0,0 +1,110 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests\Audit; + +use App\Audit\AuditRecordType; +use App\Entity\SecurityAdvisory; +use App\SecurityAdvisory\GitHubSecurityAdvisoriesSource; +use App\SecurityAdvisory\RemoteSecurityAdvisory; +use Doctrine\DBAL\Connection; +use Doctrine\Persistence\ManagerRegistry; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; + +class SecurityAdvisoryAuditRecordTest extends KernelTestCase +{ + protected function setUp(): void + { + self::bootKernel(); + static::getContainer()->get(Connection::class)->beginTransaction(); + + parent::setUp(); + } + + protected function tearDown(): void + { + static::getContainer()->get(Connection::class)->rollBack(); + + parent::tearDown(); + } + + public function testSecurityAdvisoryChangesGetRecorded(): void + { + $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + + // Created + $advisory = new SecurityAdvisory($this->remoteAdvisory('GHSA-aaaa-bbbb-cccc'), GitHubSecurityAdvisoriesSource::SOURCE_NAME); + $em->persist($advisory); + $em->flush(); + + self::assertSame(1, $this->auditCount(AuditRecordType::SecurityAdvisoryCreated)); + $attributes = $this->latestAttributes(AuditRecordType::SecurityAdvisoryCreated); + self::assertSame('acme/package', $attributes['packageName']); + self::assertSame('GHSA-aaaa-bbbb-cccc', $attributes['remoteId']); + self::assertSame('automation', $attributes['actor']); + + // Edited + $advisory->updateAdvisory($this->remoteAdvisory('GHSA-aaaa-bbbb-cccc', '^2.0')); + $em->flush(); + + self::assertSame(1, $this->auditCount(AuditRecordType::SecurityAdvisoryEdited)); + $attributes = $this->latestAttributes(AuditRecordType::SecurityAdvisoryEdited); + self::assertArrayHasKey('affectedVersions', $attributes['changes']); + self::assertSame('^1.0', $attributes['changes']['affectedVersions']['from']); + self::assertSame('^2.0', $attributes['changes']['affectedVersions']['to']); + self::assertArrayNotHasKey('updatedAt', $attributes['changes'], 'The bookkeeping timestamp should be excluded from the recorded changes'); + + // Withdrawn: deleting the advisory records a withdrawal in the transparency log. + $em->remove($advisory); + $em->flush(); + + self::assertSame(1, $this->auditCount(AuditRecordType::SecurityAdvisoryWithdrawn)); + } + + private function auditCount(AuditRecordType $type): int + { + $connection = static::getContainer()->get(Connection::class); + \assert($connection instanceof Connection); + + return (int) $connection->fetchOne('SELECT COUNT(*) FROM audit_log WHERE type = ?', [$type->value]); + } + + /** + * @return array + */ + private function latestAttributes(AuditRecordType $type): array + { + $connection = static::getContainer()->get(Connection::class); + \assert($connection instanceof Connection); + + $attributes = $connection->fetchOne('SELECT attributes FROM audit_log WHERE type = ? ORDER BY datetime DESC, id DESC LIMIT 1', [$type->value]); + + return json_decode((string) $attributes, true, flags: \JSON_THROW_ON_ERROR); + } + + private function remoteAdvisory(string $remoteId, string $affectedVersions = '^1.0'): RemoteSecurityAdvisory + { + return new RemoteSecurityAdvisory( + $remoteId, + 'Advisory title', + 'acme/package', + $affectedVersions, + 'https://example.org/'.$remoteId, + 'CVE-2024-12345', + new \DateTimeImmutable('2024-01-01 00:00:00'), + null, + [], + GitHubSecurityAdvisoriesSource::SOURCE_NAME, + null, + ); + } +} diff --git a/translations/messages.en.yml b/translations/messages.en.yml index 03a599b41..7ee94db59 100644 --- a/translations/messages.en.yml +++ b/translations/messages.en.yml @@ -207,6 +207,9 @@ audit_log: filter_list_entry_disabled: Entry disabled filter_list_entry_enabled: Entry enabled filter_list_entry_edited: Entry edited + security_advisory_created: Security advisory created + security_advisory_edited: Security advisory edited + security_advisory_withdrawn: Security advisory withdrawn organization_created: Organization created enums: user-registration-method: From 8d89e8c1a19cfe2c8290999c3b024e362db9b38b Mon Sep 17 00:00:00 2001 From: Stephan Vock Date: Fri, 10 Jul 2026 09:23:53 +0100 Subject: [PATCH 2/2] AuditRecord: drop security advisory duplicate package name and add packageId --- src/Audit/Display/AuditLogDisplayFactory.php | 6 +++--- src/Entity/AuditRecord.php | 12 ++++++----- .../SecurityAdvisoryAuditListener.php | 12 ++++++++--- .../Display/AuditLogDisplayFactoryTest.php | 6 +++--- .../Audit/SecurityAdvisoryAuditRecordTest.php | 21 ++++++++++++++++++- 5 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/Audit/Display/AuditLogDisplayFactory.php b/src/Audit/Display/AuditLogDisplayFactory.php index a219c82d2..33b228398 100644 --- a/src/Audit/Display/AuditLogDisplayFactory.php +++ b/src/Audit/Display/AuditLogDisplayFactory.php @@ -258,7 +258,7 @@ public function buildSingle(AuditRecord $record): AuditLogDisplayInterface ), AuditRecordType::SecurityAdvisoryCreated => new SecurityAdvisoryCreatedDisplay( $record->datetime, - $record->attributes['packageName'], + $record->attributes['name'], $record->attributes['advisoryId'], $record->attributes['cve'] ?? null, $record->attributes['title'], @@ -268,7 +268,7 @@ public function buildSingle(AuditRecord $record): AuditLogDisplayInterface ), AuditRecordType::SecurityAdvisoryEdited => new SecurityAdvisoryEditedDisplay( $record->datetime, - $record->attributes['packageName'], + $record->attributes['name'], $record->attributes['advisoryId'], $record->attributes['cve'] ?? null, $record->attributes['title'], @@ -279,7 +279,7 @@ public function buildSingle(AuditRecord $record): AuditLogDisplayInterface ), AuditRecordType::SecurityAdvisoryWithdrawn => new SecurityAdvisoryWithdrawnDisplay( $record->datetime, - $record->attributes['packageName'], + $record->attributes['name'], $record->attributes['advisoryId'], $record->attributes['cve'] ?? null, $record->attributes['title'], diff --git a/src/Entity/AuditRecord.php b/src/Entity/AuditRecord.php index 3bc73a8c6..8e107ea6e 100644 --- a/src/Entity/AuditRecord.php +++ b/src/Entity/AuditRecord.php @@ -515,36 +515,39 @@ public static function filterListEntryEdited(FilterListEntry $entry, array $prev ); } - public static function securityAdvisoryCreated(SecurityAdvisory $advisory, ?User $actor): self + public static function securityAdvisoryCreated(SecurityAdvisory $advisory, ?User $actor, ?int $packageId): self { return new self( AuditRecordType::SecurityAdvisoryCreated, self::getSecurityAdvisoryData($advisory, $actor), vendor: self::vendorFromPackageName($advisory->getPackageName()), actorId: $actor?->getId(), + packageId: $packageId, ); } /** * @param array> $changeSet the Doctrine entity change set (field => [old, new]) */ - public static function securityAdvisoryEdited(SecurityAdvisory $advisory, ?User $actor, array $changeSet): self + public static function securityAdvisoryEdited(SecurityAdvisory $advisory, ?User $actor, array $changeSet, ?int $packageId): self { return new self( AuditRecordType::SecurityAdvisoryEdited, [...self::getSecurityAdvisoryData($advisory, $actor), 'changes' => self::getSecurityAdvisoryChanges($changeSet)], vendor: self::vendorFromPackageName($advisory->getPackageName()), actorId: $actor?->getId(), + packageId: $packageId, ); } - public static function securityAdvisoryWithdrawn(SecurityAdvisory $advisory, ?User $actor): self + public static function securityAdvisoryWithdrawn(SecurityAdvisory $advisory, ?User $actor, ?int $packageId): self { return new self( AuditRecordType::SecurityAdvisoryWithdrawn, self::getSecurityAdvisoryData($advisory, $actor), vendor: self::vendorFromPackageName($advisory->getPackageName()), actorId: $actor?->getId(), + packageId: $packageId, ); } @@ -561,14 +564,13 @@ private static function getUserData(?User $user, string $fallbackString = 'unkno } /** - * @return array{name: string, advisoryId: string, packageName: string, source: string, remoteId: string, cve: string|null, title: string, actor: array{id: int, username: string}|string} + * @return array{name: string, advisoryId: string, source: string, remoteId: string, cve: string|null, title: string, actor: array{id: int, username: string}|string} */ private static function getSecurityAdvisoryData(SecurityAdvisory $advisory, ?User $actor): array { return [ 'name' => $advisory->getPackageName(), 'advisoryId' => $advisory->getPackagistAdvisoryId(), - 'packageName' => $advisory->getPackageName(), 'source' => $advisory->getSource(), 'remoteId' => $advisory->getRemoteId(), 'cve' => $advisory->getCve(), diff --git a/src/EventListener/SecurityAdvisoryAuditListener.php b/src/EventListener/SecurityAdvisoryAuditListener.php index aea15b637..31a021040 100644 --- a/src/EventListener/SecurityAdvisoryAuditListener.php +++ b/src/EventListener/SecurityAdvisoryAuditListener.php @@ -13,6 +13,7 @@ namespace App\EventListener; use App\Entity\AuditRecord; +use App\Entity\Package; use App\Entity\SecurityAdvisory; use App\Entity\User; use App\Util\DoctrineTrait; @@ -48,12 +49,12 @@ public function __construct( */ public function postPersist(SecurityAdvisory $advisory, LifecycleEventArgs $event): void { - $this->getEM()->getRepository(AuditRecord::class)->insert(AuditRecord::securityAdvisoryCreated($advisory, $this->getUser())); + $this->getEM()->getRepository(AuditRecord::class)->insert(AuditRecord::securityAdvisoryCreated($advisory, $this->getUser(), $this->getPackageId($advisory))); } public function preUpdate(SecurityAdvisory $advisory, PreUpdateEventArgs $event): void { - $this->buffered[] = AuditRecord::securityAdvisoryEdited($advisory, $this->getUser(), $event->getEntityChangeSet()); + $this->buffered[] = AuditRecord::securityAdvisoryEdited($advisory, $this->getUser(), $event->getEntityChangeSet(), $this->getPackageId($advisory)); } /** @@ -77,7 +78,7 @@ public function postUpdate(SecurityAdvisory $advisory, LifecycleEventArgs $event */ public function preRemove(SecurityAdvisory $advisory, LifecycleEventArgs $event): void { - $this->getEM()->persist(AuditRecord::securityAdvisoryWithdrawn($advisory, $this->getUser())); + $this->getEM()->persist(AuditRecord::securityAdvisoryWithdrawn($advisory, $this->getUser(), $this->getPackageId($advisory))); } private function getUser(): ?User @@ -86,4 +87,9 @@ private function getUser(): ?User return $user instanceof User ? $user : null; } + + private function getPackageId(SecurityAdvisory $advisory): ?int + { + return $this->getEM()->getRepository(Package::class)->getPackageIdByName($advisory->getPackageName()); + } } diff --git a/tests/Audit/Display/AuditLogDisplayFactoryTest.php b/tests/Audit/Display/AuditLogDisplayFactoryTest.php index 2bc614179..13b94ecee 100644 --- a/tests/Audit/Display/AuditLogDisplayFactoryTest.php +++ b/tests/Audit/Display/AuditLogDisplayFactoryTest.php @@ -645,7 +645,7 @@ public function testBuildSecurityAdvisoryCreated(): void AuditRecordType::SecurityAdvisoryCreated, [ 'advisoryId' => 'PKSA-abcd-1234-5678', - 'packageName' => 'acme/package', + 'name' => 'acme/package', 'source' => 'GitHub', 'remoteId' => 'GHSA-aaaa-bbbb-cccc', 'cve' => 'CVE-2024-12345', @@ -675,7 +675,7 @@ public function testBuildSecurityAdvisoryEdited(): void AuditRecordType::SecurityAdvisoryEdited, [ 'advisoryId' => 'PKSA-abcd-1234-5678', - 'packageName' => 'acme/package', + 'name' => 'acme/package', 'source' => 'GitHub', 'remoteId' => 'GHSA-aaaa-bbbb-cccc', 'cve' => 'CVE-2024-12345', @@ -699,7 +699,7 @@ public function testBuildSecurityAdvisoryWithdrawn(): void AuditRecordType::SecurityAdvisoryWithdrawn, [ 'advisoryId' => 'PKSA-abcd-1234-5678', - 'packageName' => 'acme/package', + 'name' => 'acme/package', 'source' => 'GitHub', 'remoteId' => 'GHSA-aaaa-bbbb-cccc', 'cve' => null, diff --git a/tests/Audit/SecurityAdvisoryAuditRecordTest.php b/tests/Audit/SecurityAdvisoryAuditRecordTest.php index 61ca34026..9c0ab2968 100644 --- a/tests/Audit/SecurityAdvisoryAuditRecordTest.php +++ b/tests/Audit/SecurityAdvisoryAuditRecordTest.php @@ -16,12 +16,15 @@ use App\Entity\SecurityAdvisory; use App\SecurityAdvisory\GitHubSecurityAdvisoriesSource; use App\SecurityAdvisory\RemoteSecurityAdvisory; +use App\Tests\Fixtures\Fixtures; use Doctrine\DBAL\Connection; use Doctrine\Persistence\ManagerRegistry; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; class SecurityAdvisoryAuditRecordTest extends KernelTestCase { + use Fixtures; + protected function setUp(): void { self::bootKernel(); @@ -41,6 +44,11 @@ public function testSecurityAdvisoryChangesGetRecorded(): void { $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + // The advisory targets a package that is available on Packagist so the audit record can be linked to it. + $package = self::createPackage('acme/package', 'https://example.org/acme/package.git'); + $em->persist($package); + $em->flush(); + // Created $advisory = new SecurityAdvisory($this->remoteAdvisory('GHSA-aaaa-bbbb-cccc'), GitHubSecurityAdvisoriesSource::SOURCE_NAME); $em->persist($advisory); @@ -48,9 +56,10 @@ public function testSecurityAdvisoryChangesGetRecorded(): void self::assertSame(1, $this->auditCount(AuditRecordType::SecurityAdvisoryCreated)); $attributes = $this->latestAttributes(AuditRecordType::SecurityAdvisoryCreated); - self::assertSame('acme/package', $attributes['packageName']); + self::assertSame('acme/package', $attributes['name']); self::assertSame('GHSA-aaaa-bbbb-cccc', $attributes['remoteId']); self::assertSame('automation', $attributes['actor']); + self::assertSame($package->getId(), $this->latestPackageId(AuditRecordType::SecurityAdvisoryCreated)); // Edited $advisory->updateAdvisory($this->remoteAdvisory('GHSA-aaaa-bbbb-cccc', '^2.0')); @@ -91,6 +100,16 @@ private function latestAttributes(AuditRecordType $type): array return json_decode((string) $attributes, true, flags: \JSON_THROW_ON_ERROR); } + private function latestPackageId(AuditRecordType $type): ?int + { + $connection = static::getContainer()->get(Connection::class); + \assert($connection instanceof Connection); + + $packageId = $connection->fetchOne('SELECT packageId FROM audit_log WHERE type = ? ORDER BY datetime DESC, id DESC LIMIT 1', [$type->value]); + + return $packageId === false || $packageId === null ? null : (int) $packageId; + } + private function remoteAdvisory(string $remoteId, string $affectedVersions = '^1.0'): RemoteSecurityAdvisory { return new RemoteSecurityAdvisory(