From 12851d11822f96e826fed3e2684ac147c69b04eb Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 27 May 2026 10:45:28 +0200 Subject: [PATCH 01/13] Make stable versions immutable and allow soft-deletion / hiding of versions --- config/routes.yaml | 6 + css/app.scss | 49 ++- js/view.js | 107 ++++++- migrations/2026_05_version_immutability.sql | 20 ++ phpstan-baseline.neon | 2 +- phpstan.neon | 4 +- src/Audit/AuditRecordType.php | 6 +- src/Audit/Display/AuditLogDisplayFactory.php | 26 ++ src/Audit/Display/VersionRecoveredDisplay.php | 39 +++ .../VersionReferenceChangeBlockedDisplay.php | 40 +++ .../Display/VersionSoftDeletedDisplay.php | 40 +++ src/Audit/VersionDeletionReason.php | 89 ++++++ src/Command/ClearVersionsCommand.php | 8 + src/Command/UnfreezePackageCommand.php | 10 + src/Command/UpdatePackagesCommand.php | 12 +- src/Controller/ApiController.php | 2 +- src/Controller/PackageController.php | 151 ++++++++- src/Controller/UserController.php | 8 +- src/DataFixtures/PackageFixtures.php | 2 +- src/Entity/AuditRecord.php | 37 +++ src/Entity/Version.php | 64 ++++ src/Entity/VersionRepository.php | 76 ++++- src/Model/PackageManager.php | 48 +++ src/Package/Updater.php | 298 +++++++++++++++--- src/Package/V2Dumper.php | 7 +- src/Security/Voter/PackageActions.php | 3 + src/Security/Voter/PackageVoter.php | 14 +- src/Service/Scheduler.php | 10 +- src/Service/UpdaterWorker.php | 4 +- .../about/version_immutability.html.twig | 37 +++ .../display/version_recovered.html.twig | 5 + ...version_reference_change_blocked.html.twig | 6 + .../display/version_soft_deleted.html.twig | 5 + .../version_reference_change_blocked.txt.twig | 26 ++ templates/package/version_list.html.twig | 62 +++- templates/package/view_package.html.twig | 5 +- tests/Entity/VersionRepositoryTest.php | 107 +++++++ tests/Package/UpdaterTest.php | 280 +++++++++++++++- tests/Package/V2DumperTest.php | 22 ++ translations/messages.en.yml | 3 + 40 files changed, 1647 insertions(+), 93 deletions(-) create mode 100644 migrations/2026_05_version_immutability.sql create mode 100644 src/Audit/Display/VersionRecoveredDisplay.php create mode 100644 src/Audit/Display/VersionReferenceChangeBlockedDisplay.php create mode 100644 src/Audit/Display/VersionSoftDeletedDisplay.php create mode 100644 src/Audit/VersionDeletionReason.php create mode 100644 templates/about/version_immutability.html.twig create mode 100644 templates/audit_log/display/version_recovered.html.twig create mode 100644 templates/audit_log/display/version_reference_change_blocked.html.twig create mode 100644 templates/audit_log/display/version_soft_deleted.html.twig create mode 100644 templates/email/version_reference_change_blocked.txt.twig diff --git a/config/routes.yaml b/config/routes.yaml index afe930bbf..dfb99dbde 100644 --- a/config/routes.yaml +++ b/config/routes.yaml @@ -16,6 +16,12 @@ about: defaults: template: 'about/about.html.twig' +about_version_immutability: + path: '/about/version-immutability' + controller: 'Symfony\Bundle\FrameworkBundle\Controller\TemplateController' + defaults: + template: 'about/version_immutability.html.twig' + about_composer: path: '/about-composer' controller: 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction' diff --git a/css/app.scss b/css/app.scss index b4c22d352..873efd958 100644 --- a/css/app.scss +++ b/css/app.scss @@ -1314,11 +1314,14 @@ input:focus:invalid:focus, textarea:focus:invalid:focus, select:focus:invalid:fo margin-right: 4px; } .package .package-aside a.advisory-alert { - margin-left: -20px; + margin-left: -40px; } .package .package-aside a.malware-alert { margin-left: -40px; } +.package .package-aside a.retag-blocked-alert { + margin-left: -40px; +} .package .package-aside a.action-alert:hover, .package .package-aside a.action-alert:active { text-decoration: none; } @@ -1385,17 +1388,54 @@ input:focus:invalid:focus, textarea:focus:invalid:focus, select:focus:invalid:fo display: block; height: 26px; } -.package .delete-version { +.package .delete-version, .package .hide-version { display: inline-block; } -.package .delete-version .submit { +.package .delete-version .submit, .package .hide-version .submit { position: absolute; - right: 5px; top: 7px; + cursor: pointer; } +.package .delete-version .submit { right: 5px; } +.package .hide-version .submit { right: 25px; color: #999; } .package .delete-version .submit:hover, .package .package-aside a.action-alert:hover i { color: #cd3729; } +.package .hide-version .submit:hover { + color: #b87800; +} + +.package .versions .version.version-soft-deleted .version-number { + color: #999; + text-decoration: line-through; + text-decoration-color: rgba(153, 153, 153, 0.5); +} +.package .package-aside .deletion-alert { + position: absolute; + right: 28px; + top: 2px; + cursor: help; +} +.package .package-aside .deletion-alert i { + color: #999; + font-size: 12px; +} +.package .package-aside a.retag-blocked-alert i { + color: #b87800; +} +.package .recover-version { + display: inline-block; +} +.package .recover-version .submit { + position: absolute; + right: 5px; + top: 6px; + cursor: pointer; + color: #999; +} +.package .recover-version .submit:hover { + color: #3c763d; +} @media (min-width: 992px) { .search-facets .ais-Menu-count, .search-facets .ais-RefinementList-count { @@ -1427,6 +1467,7 @@ input:focus:invalid:focus, textarea:focus:invalid:focus, select:focus:invalid:fo .package .delete-version .submit { right: 15px; } + .package .hide-version .submit { right: 35px; } } @media (max-width: 767px) { .package-aside .facts { diff --git a/js/view.js b/js/view.js index 558916774..20842ece0 100644 --- a/js/view.js +++ b/js/view.js @@ -191,23 +191,118 @@ const init = function ($) { const details = e.target.dataset.details; notifier.log(message, {}, details); }); - $('.package .delete-version .submit').on('click', function (e) { + $('.package .delete-version .submit, .package .recover-version .submit, .package .hide-version .submit').on('click', function (e) { e.preventDefault(); e.stopImmediatePropagation(); $(e.target).closest('form').submit(); }); + function getVersionLabel(form) { + return $(form).closest('.version').find('.version-number').text().trim(); + } + + function applyVersionDeleteResponse(form, data, deletedToast) { + var row = $(form).closest('.version'); + if (data && data.softDeleted) { + notifier.log('Version soft-deleted. Reload the page to access the recovery action.', {timeout: 4000}); + row.addClass('version-soft-deleted'); + if (!row.find('.deletion-alert').length) { + var icon = data.deletionIcon || 'glyphicon-trash'; + var alert = $(''); + alert.find('i').addClass(icon); + alert.attr('title', data.deletionTitle || 'Deleted'); + alert.insertBefore(row.find('form').first()); + } + row.find('.delete-version, .hide-version').remove(); + } else { + notifier.log(deletedToast, {timeout: 3000}); + row.remove(); + } + } + + // Submit a version-action form via ajax, guarding against duplicate submits. + // `overrides` may carry {url, type, data} to override the form's defaults (used by the + // admin-reason fallthrough in .delete-version which retargets to admin_delete_version). + // Returns the jqXHR, or null if the request-sent guard tripped. + function dispatchVersionAction(form, onSuccess, overrides) { + if ($(form).is('.request-sent')) { + return null; + } + overrides = overrides || {}; + $(form).addClass('request-sent'); + return $.ajax({ + url: overrides.url || $(form).attr('action'), + type: overrides.type || $(form).attr('method'), + cache: false, + dataType: 'json', + data: overrides.data || $(form).serializeArray(), + success: onSuccess, + complete: function () { $(form).removeClass('request-sent'); } + }); + } + + $('.package .recover-version').on('submit', function (e) { + e.preventDefault(); + e.stopImmediatePropagation(); + var form = this; + dispatchVersionAction(form, function () { + notifier.log('Version recovered. Reload the page to see the active version.', {timeout: 3000}); + var row = $(form).closest('.version'); + row.removeClass('version-soft-deleted'); + row.find('.deletion-alert').remove(); + $(form).remove(); + }); + }); + $('.package .delete-version').on('submit', function (e) { e.preventDefault(); e.stopImmediatePropagation(); var form = this; - if (window.confirm('Are you sure you want to delete ' + $(form).prev().text() + '?')) { - dispatchAjaxForm(this, function () { - notifier.log('Version successfully deleted', {timeout: 3000}); - $(form).closest('.version').remove(); - }, 'request-sent'); + var label = getVersionLabel(form); + var overrides = {}; + + if ($(form).data('admin')) { + var reason = window.prompt('Reason text for admin removal of ' + label + ' (leave blank to record without a reason, cancel to abort):', ''); + if (reason === null) { + return; + } + reason = reason.trim(); + if (reason !== '') { + overrides.url = $(form).data('admin-url'); + overrides.type = 'POST'; + overrides.data = [ + {name: '_token', value: $(form).find('input[name="_token"]').val()}, + {name: 'reason', value: reason} + ]; + } + } else if (!window.confirm('Are you sure you want to delete ' + label + '?')) { + return; + } + + dispatchVersionAction(form, function (data) { + applyVersionDeleteResponse(form, data, 'Version successfully deleted'); + }, overrides); + }); + + $('.package .hide-version').on('submit', function (e) { + e.preventDefault(); + e.stopImmediatePropagation(); + var form = this; + var label = getVersionLabel(form); + var reason = window.prompt('Reason text for hiding ' + label + ' from public (leave blank to record without a reason, cancel to abort):', ''); + if (reason === null) { + return; } + reason = reason.trim(); + var data = $(form).serializeArray(); + if (reason !== '') { + data.push({name: 'reason', value: reason}); + } + dispatchVersionAction(form, function (resp) { + applyVersionDeleteResponse(form, resp, 'Version hidden'); + }, {data: data}); }); + $('.package').on('click', '.requireme input', function () { this.select(); }); diff --git a/migrations/2026_05_version_immutability.sql b/migrations/2026_05_version_immutability.sql new file mode 100644 index 000000000..913d4e3ee --- /dev/null +++ b/migrations/2026_05_version_immutability.sql @@ -0,0 +1,20 @@ +-- Version immutability + soft-delete reasons +-- +-- Adds: +-- deletionReason enum-string column (auto_missing | maintainer | admin) +-- deletionReasonText optional human-readable reason for admin takedowns +-- lastBlockedReference last attempted source/dist ref that was refused on a stable version +-- index over (softDeletedAt, deletionReason) to keep the purge sweep fast +-- +-- The UPDATE seeds existing softDeletedAt rows with the auto_missing reason so the +-- invariant "softDeletedAt IS NOT NULL ⇔ deletionReason IS NOT NULL" holds everywhere. + +ALTER TABLE package_version + ADD COLUMN deletionReason VARCHAR(32) NULL AFTER softDeletedAt, + ADD COLUMN deletionReasonText TEXT NULL AFTER deletionReason, + ADD COLUMN lastBlockedReference VARCHAR(255) NULL AFTER deletionReasonText, + ADD INDEX softdel_reason_idx (softDeletedAt, deletionReason); + +UPDATE package_version + SET deletionReason = 'auto_missing' + WHERE softDeletedAt IS NOT NULL; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 46738f7c8..6e4ee5576 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -103,7 +103,7 @@ parameters: path: src/Package/Updater.php - - message: '#^Parameter \#1 \$job of method App\\Service\\UpdaterWorker\:\:process\(\) expects App\\Entity\\Job\\|App\\Entity\\Job\\|App\\Entity\\Job\, App\\Entity\\Job\\> given\.$#' + message: '#^Parameter \#1 \$job of method App\\Service\\UpdaterWorker\:\:process\(\) expects App\\Entity\\Job\\|App\\Entity\\Job\\|App\\Entity\\Job\, App\\Entity\\Job\\> given\.$#' identifier: argument.type count: 1 path: src/Service/QueueWorker.php diff --git a/phpstan.neon b/phpstan.neon index ba362f066..03d71438a 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -50,14 +50,14 @@ parameters: VersionData: 'array}>' VersionDataLinks: 'array' - ExistingVersionsForUpdate: 'array' + ExistingVersionsForUpdate: 'array' PackageAutoloadRules: 'array{psr-0?: array, psr-4?: array, classmap?: list, files?: list, exclude-from-classmap?: list}' PackageDist: 'array{url: string|null, type: string|null, reference: string|null, shasum: string|null}|null' PackageSource: 'array{url: string|null, type: string|null, reference: string|null}|null' AnyJob: 'array' - PackageUpdateJob: 'array{id: int, update_equal_refs: bool, delete_before: bool, force_dump: bool, source: string}' + PackageUpdateJob: 'array{id: int, update_source_dist_url: bool, delete_before: bool, force_dump: bool, source: string}' GitHubUserMigrateJob: 'array{id: int, old_scope: string, new_scope: string}' SecurityAdvisoryJob: 'array{source: string}' FilterListJob: 'array{list: string, source: string}' diff --git a/src/Audit/AuditRecordType.php b/src/Audit/AuditRecordType.php index 73d772457..67efe9406 100644 --- a/src/Audit/AuditRecordType.php +++ b/src/Audit/AuditRecordType.php @@ -31,7 +31,10 @@ enum AuditRecordType: string // version case VersionCreated = 'version_created'; case VersionReferenceChanged = 'version_reference_changed'; + case VersionReferenceChangeBlocked = 'version_reference_change_blocked'; case VersionDeleted = 'version_deleted'; + case VersionSoftDeleted = 'version_soft_deleted'; + case VersionRecovered = 'version_recovered'; // user management case UserCreated = 'user_created'; @@ -57,7 +60,8 @@ public function category(): string self::MaintainerAdded, self::MaintainerRemoved, self::PackageTransferred => 'ownership', self::PackageCreated, self::PackageDeleted, self::CanonicalUrlChanged, self::PackageAbandoned, self::PackageUnabandoned, self::PackageFrozen, self::PackageUnfrozen => 'package', - self::VersionCreated, self::VersionDeleted, self::VersionReferenceChanged => 'version', + self::VersionCreated, self::VersionDeleted, self::VersionReferenceChanged, + self::VersionReferenceChangeBlocked, self::VersionSoftDeleted, self::VersionRecovered => 'version', self::UserCreated, self::UserVerified, self::UserDeleted, self::PasswordResetRequested, self::PasswordReset, self::PasswordChanged, self::EmailChanged, self::UsernameChanged, self::GitHubLinkedWithUser, diff --git a/src/Audit/Display/AuditLogDisplayFactory.php b/src/Audit/Display/AuditLogDisplayFactory.php index b2a7a3101..d313e3649 100644 --- a/src/Audit/Display/AuditLogDisplayFactory.php +++ b/src/Audit/Display/AuditLogDisplayFactory.php @@ -147,6 +147,32 @@ public function buildSingle(AuditRecord $record): AuditLogDisplayInterface $this->buildActor($record->attributes['actor'] ?? null), $record->ip, ), + AuditRecordType::VersionReferenceChangeBlocked => new VersionReferenceChangeBlockedDisplay( + $record->datetime, + $record->attributes['name'], + $record->attributes['version'], + $record->attributes['ref_from'] ?? null, + $record->attributes['ref_to'], + $this->buildActor($record->attributes['actor'] ?? null), + $record->ip, + ), + AuditRecordType::VersionSoftDeleted => new VersionSoftDeletedDisplay( + $record->datetime, + $record->attributes['name'], + $record->attributes['version'], + $record->attributes['reason'], + $record->attributes['reasonText'] ?? null, + $this->buildActor($record->attributes['actor'] ?? null), + $record->ip, + ), + AuditRecordType::VersionRecovered => new VersionRecoveredDisplay( + $record->datetime, + $record->attributes['name'], + $record->attributes['version'], + $record->attributes['previousReason'], + $this->buildActor($record->attributes['actor'] ?? null), + $record->ip, + ), AuditRecordType::UserCreated => new UserCreatedDisplay( $record->datetime, $record->attributes['user']['username'], diff --git a/src/Audit/Display/VersionRecoveredDisplay.php b/src/Audit/Display/VersionRecoveredDisplay.php new file mode 100644 index 000000000..063e1d0e4 --- /dev/null +++ b/src/Audit/Display/VersionRecoveredDisplay.php @@ -0,0 +1,39 @@ + + * 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 VersionRecoveredDisplay extends AbstractAuditLogDisplay +{ + public function __construct( + \DateTimeImmutable $datetime, + public string $packageName, + public string $version, + public string $previousReason, + ActorDisplay $actor, + ?string $ip, + ) { + parent::__construct($datetime, $actor, $ip); + } + + public function getType(): AuditRecordType + { + return AuditRecordType::VersionRecovered; + } + + public function getTemplateName(): string + { + return 'audit_log/display/version_recovered.html.twig'; + } +} diff --git a/src/Audit/Display/VersionReferenceChangeBlockedDisplay.php b/src/Audit/Display/VersionReferenceChangeBlockedDisplay.php new file mode 100644 index 000000000..2c5a651d3 --- /dev/null +++ b/src/Audit/Display/VersionReferenceChangeBlockedDisplay.php @@ -0,0 +1,40 @@ + + * 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 VersionReferenceChangeBlockedDisplay extends AbstractAuditLogDisplay +{ + public function __construct( + \DateTimeImmutable $datetime, + public string $packageName, + public string $version, + public ?string $refFrom, + public string $refTo, + ActorDisplay $actor, + ?string $ip, + ) { + parent::__construct($datetime, $actor, $ip); + } + + public function getType(): AuditRecordType + { + return AuditRecordType::VersionReferenceChangeBlocked; + } + + public function getTemplateName(): string + { + return 'audit_log/display/version_reference_change_blocked.html.twig'; + } +} diff --git a/src/Audit/Display/VersionSoftDeletedDisplay.php b/src/Audit/Display/VersionSoftDeletedDisplay.php new file mode 100644 index 000000000..59af7f0fc --- /dev/null +++ b/src/Audit/Display/VersionSoftDeletedDisplay.php @@ -0,0 +1,40 @@ + + * 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 VersionSoftDeletedDisplay extends AbstractAuditLogDisplay +{ + public function __construct( + \DateTimeImmutable $datetime, + public string $packageName, + public string $version, + public string $reason, + public ?string $reasonText, + ActorDisplay $actor, + ?string $ip, + ) { + parent::__construct($datetime, $actor, $ip); + } + + public function getType(): AuditRecordType + { + return AuditRecordType::VersionSoftDeleted; + } + + public function getTemplateName(): string + { + return 'audit_log/display/version_soft_deleted.html.twig'; + } +} diff --git a/src/Audit/VersionDeletionReason.php b/src/Audit/VersionDeletionReason.php new file mode 100644 index 000000000..e4d13859e --- /dev/null +++ b/src/Audit/VersionDeletionReason.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\Audit; + +/** + * A version that has been soft-deleted with one of these reasons is a stable, published snapshot + * whose identity (source/dist reference) is frozen. Hard deletion of such a row is NOT and MUST NOT + * be supported — not via the UI, not via admin tooling, not via CLI commands. + * + * The reason is structural: hard-deleting a stable version frees its (package, version-string) slot + * and would let a subsequent crawl create a new row at the same coordinates with different content, + * breaking the immutability guarantee that downstream tooling (Composer's lock-file pinning, + * security scanners, mirrors) relies on. Soft-deleted rows therefore persist indefinitely as + * immutable historical records; the V2 dump filters them out, but the database row stays. + * + * Dev versions (branches like `dev-main`) are NOT subject to this rule — they are mutable trackers + * by design, never carry a reason from this enum during normal operation (apart from + * AutoDeletedMissing while the branch is temporarily gone), and may be hard-deleted by the Updater + * housekeeping or the per-version UI delete. The auto-purge in Updater::update() only fires on + * dev rows; stable AutoDeletedMissing rows stay until they reappear upstream and auto-recover. + * + * Whole-package deletion (PackageManager::deletePackage / CleanSpamPackagesCommand) is a separate + * concept — the entire package goes away, versions cascade with it — and is out of scope here. + */ +enum VersionDeletionReason: string +{ + /** + * Auto-soft-deleted by the Updater when a version disappears from upstream. + * - Metadata: filtered out of V2 dumps. + * - Page: shown grayed-out to every visitor. + * - Recovery: anyone with DeleteVersion permission; the Updater also auto-recovers on + * reappearance. + */ + case AutoDeletedMissing = 'auto_missing'; + + /** + * Maintainer pulled the version via the UI. + * - Metadata: filtered out of V2 dumps. + * - Page: shown grayed-out to every visitor. + * - Recovery: maintainer or admin. The Updater never recreates it. + */ + case DeletedByMaintainer = 'maintainer'; + + /** + * Admin pulled the version (e.g. bad release, supply-chain concern). + * - Metadata: filtered out of V2 dumps. + * - Page: shown grayed-out to every visitor (with reason text if provided). + * - Recovery: admin only. The Updater never recreates it. + */ + case DeletedByAdmin = 'admin'; + + /** + * Admin took the version down with no public trace — used by the spam-cascade flow + * (reason text 'spam') and for selective per-version takedowns where the public should + * not see the row at all. The admin is prompted for a reason text in the per-version UI. + * - Metadata: filtered out of V2 dumps. + * - Page: hidden from non-maintainers; shown grayed-out to maintainers and admins. + * - Recovery: admin only. Unfreezing a spam-frozen package also bulk-recovers Hidden + * versions of that package. + */ + case Hidden = 'hidden'; + + public function isRecoverableByMaintainer(): bool + { + return match ($this) { + self::DeletedByMaintainer, self::AutoDeletedMissing => true, + self::DeletedByAdmin, self::Hidden => false, + }; + } + + /** + * Whether the soft-deleted version should still appear on the package page for the general + * public. Maintainers and admins see all soft-deleted versions regardless. + */ + public function isVisibleToPublic(): bool + { + return $this !== self::Hidden; + } +} diff --git a/src/Command/ClearVersionsCommand.php b/src/Command/ClearVersionsCommand.php index 63c6b1afb..7bda43db4 100644 --- a/src/Command/ClearVersionsCommand.php +++ b/src/Command/ClearVersionsCommand.php @@ -68,6 +68,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int foreach ($versions as $version) { $name = $version->getName().' '.$version->getVersion(); + if (!$version->isDevelopment()) { + $output->writeln('Skipping stable version '.$name.' (stable releases are immutable and cannot be hard-deleted)'); + continue; + } $output->writeln('Clearing '.$name); if ($force) { $packageNames[] = $version->getName(); @@ -95,6 +99,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int foreach ($versions as $version) { $name = $version->getName().' '.$version->getVersion(); + if (!$version->isDevelopment()) { + $output->writeln('Skipping stable version '.$name.' (stable releases are immutable and cannot be hard-deleted)'); + continue; + } $output->writeln('Clearing '.$name); if ($force) { $packageNames[] = $version->getName(); diff --git a/src/Command/UnfreezePackageCommand.php b/src/Command/UnfreezePackageCommand.php index fe0fbbba6..357de03f7 100644 --- a/src/Command/UnfreezePackageCommand.php +++ b/src/Command/UnfreezePackageCommand.php @@ -13,6 +13,9 @@ namespace App\Command; use App\Entity\Package; +use App\Entity\PackageFreezeReason; +use App\Entity\Version; +use App\Entity\VersionRepository; use App\Model\ProviderManager; use App\Service\Scheduler; use Doctrine\Persistence\ManagerRegistry; @@ -62,8 +65,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->providerManager->insertPackage($package); $package->setCrawledAt(null); $package->setUpdatedAt(new \DateTimeImmutable()); + $wasSpam = $package->getFreezeReason() === PackageFreezeReason::Spam; $package->unfreeze(); + if ($wasSpam) { + /** @var VersionRepository $versionRepo */ + $versionRepo = $this->getEM()->getRepository(Version::class); + $versionRepo->recoverHiddenVersionsForPackage($package, null); + } + $this->getEM()->flush(); $this->scheduler->scheduleUpdate($package, 'unfreeze cmd', forceDump: true); diff --git a/src/Command/UpdatePackagesCommand.php b/src/Command/UpdatePackagesCommand.php index 55a0ae552..167a18488 100644 --- a/src/Command/UpdatePackagesCommand.php +++ b/src/Command/UpdatePackagesCommand.php @@ -47,7 +47,7 @@ protected function configure(): void ->setName('packagist:update') ->setDefinition([ new InputOption('force', null, InputOption::VALUE_NONE, 'Force a re-crawl of all packages, or if a package name is given forces an update of all versions'), - new InputOption('delete-before', null, InputOption::VALUE_NONE, 'Force deletion of all versions before an update'), + new InputOption('delete-before', null, InputOption::VALUE_NONE, 'Force deletion of all dev versions before an update, stable versions remain'), new InputOption('update-equal-refs', null, InputOption::VALUE_NONE, 'Force update of all versions even when they already exist'), new InputArgument('package', InputArgument::OPTIONAL, 'Package name to update'), ]) @@ -62,7 +62,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $package = $input->getArgument('package'); $deleteBefore = false; - $updateEqualRefs = false; + $updateSourceDistUrl = false; $randomTimes = true; if (!$this->locker->lockCommand(__CLASS__)) { @@ -82,12 +82,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $packages = [['id' => $packageEntity->getId()]]; if ($force) { - $updateEqualRefs = true; + $updateSourceDistUrl = true; } $randomTimes = false; } elseif ($force) { $packages = $this->getEM()->getConnection()->fetchAllAssociative('SELECT id FROM package ORDER BY id ASC'); - $updateEqualRefs = true; + $updateSourceDistUrl = true; } else { $packages = $this->getEM()->getRepository(Package::class)->getStalePackages(); } @@ -101,14 +101,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int $deleteBefore = true; } if ($input->getOption('update-equal-refs')) { - $updateEqualRefs = true; + $updateSourceDistUrl = true; } while ($ids) { $idsGroup = array_splice($ids, 0, 100); foreach ($idsGroup as $id) { - $job = $this->scheduler->scheduleUpdate($id, 'update cmd', $updateEqualRefs, $deleteBefore, $randomTimes ? new \DateTimeImmutable('+'.random_int(1, 600).'seconds') : null); + $job = $this->scheduler->scheduleUpdate($id, 'update cmd', $updateSourceDistUrl, $deleteBefore, $randomTimes ? new \DateTimeImmutable('+'.random_int(1, 600).'seconds') : null); if ($verbose) { $output->writeln('Scheduled update job '.$job->getId().' for package '.$id); } diff --git a/src/Controller/ApiController.php b/src/Controller/ApiController.php index 8129df618..c8411b0a4 100644 --- a/src/Controller/ApiController.php +++ b/src/Controller/ApiController.php @@ -738,7 +738,7 @@ protected function findGitHubPackagesByRepository(string $path, string $remoteId $package->setRepository($url); if ($url !== $previousUrl) { // ensure we do a full update of all versions to update the repo URL - $this->scheduler->scheduleUpdate($package, $source, updateEqualRefs: true, forceDump: true); + $this->scheduler->scheduleUpdate($package, $source, updateSourceDistUrl: true, forceDump: true); } } } diff --git a/src/Controller/PackageController.php b/src/Controller/PackageController.php index 337112f1c..4f563bd3e 100644 --- a/src/Controller/PackageController.php +++ b/src/Controller/PackageController.php @@ -13,6 +13,7 @@ namespace App\Controller; use App\Audit\AbandonmentReason; +use App\Audit\VersionDeletionReason; use App\Entity\AuditRecord; use App\Entity\Dependent; use App\Entity\Download; @@ -459,7 +460,7 @@ public function markSafeAction(Request $req): RedirectResponse #[IsGranted('ROLE_ADMIN')] #[Route(path: '/package/{name}/unfreeze', name: 'unfreeze_package', requirements: ['name' => Package::PACKAGE_NAME_REGEX], defaults: ['_format' => 'html'], methods: ['POST'])] - public function unfreezePackageAction(Request $req, string $name, CsrfTokenManagerInterface $csrfTokenManager): Response + public function unfreezePackageAction(Request $req, string $name, CsrfTokenManagerInterface $csrfTokenManager, #[CurrentUser] User $user): Response { if (!$this->isCsrfTokenValid('unfreeze', $req->request->getString('token'))) { throw new BadRequestHttpException('Invalid CSRF token'); @@ -470,8 +471,14 @@ public function unfreezePackageAction(Request $req, string $name, CsrfTokenManag return $package; } + $wasSpam = $package->getFreezeReason() === PackageFreezeReason::Spam; $package->unfreeze(); $this->getEM()->persist($package); + + if ($wasSpam) { + $this->getEM()->getRepository(Version::class)->recoverHiddenVersionsForPackage($package, $user); + } + $this->getEM()->flush(); return $this->redirectToRoute('view_package', ['name' => $package->getName()]); @@ -585,6 +592,10 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn /** @var Version[] $versions */ $versions = $package->getVersions()->toArray(); + if (!$this->isGranted(PackageActions::DeleteVersion->value, $package)) { + $versions = array_values(array_filter($versions, static fn (Version $v): bool => $v->getDeletionReason()?->isVisibleToPublic() ?? true)); + } + usort($versions, Package::class.'::sortVersions'); if (\count($versions)) { @@ -698,8 +709,18 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn $data['listsFlaggingVersionsAsMalware'] = []; } - if ($this->isGranted(PackageActions::DeleteVersion->value, $package)) { - $data['deleteVersionCsrfToken'] = $csrfTokenManager->getToken('delete_version'); + // A single CSRF token covers every version-mutation action on this package. The actions + // (delete, recover, admin-delete, hide) are gated independently by voter grants on the + // template side; collapsing the token to one value avoids generating and shipping four + // separate tokens to the page. + $data['canDeleteVersion'] = $this->isGranted(PackageActions::DeleteVersion->value, $package); + // The admin-delete reason prompt is only meaningful when the actor is not a maintainer of + // this package — for admin-maintainers, the basic delete flow already attributes the action + // to them as a maintainer. + $data['canAdminDeleteVersion'] = !$package->isMaintainer($user) && $this->isGranted(PackageActions::AdminDeleteVersion->value, $package); + $data['canHideVersion'] = $this->isGranted(PackageActions::HideVersion->value, $package); + if ($data['canDeleteVersion'] || $data['canAdminDeleteVersion'] || $data['canHideVersion']) { + $data['manageVersionsCsrfToken'] = $csrfTokenManager->getToken('manage_versions'); } if ($this->isGranted(PackageActions::Update->value, $package)) { $lastJob = $this->getEM()->getRepository(Job::class)->findLatestExecutedJob($package->getId(), 'package:updates'); @@ -804,7 +825,7 @@ public function viewPackageVersionAction(Request $req, int $versionId): JsonResp } #[Route(path: '/versions/{versionId}', name: 'delete_version', requirements: ['versionId' => '[0-9]+'], methods: ['DELETE'])] - public function deletePackageVersionAction(Request $req, int $versionId): Response + public function deletePackageVersionAction(Request $req, int $versionId, #[CurrentUser] User $user): Response { $repo = $this->getEM()->getRepository(Version::class); @@ -817,11 +838,123 @@ public function deletePackageVersionAction(Request $req, int $versionId): Respon $this->denyAccessUnlessGranted(PackageActions::DeleteVersion->value, $package, 'No permission to delete versions'); - if (!$this->isCsrfTokenValid('delete_version', $req->request->getString('_token'))) { + if (!$this->isCsrfTokenValid('manage_versions', $req->request->getString('_token'))) { + throw new AccessDeniedException('Invalid CSRF token'); + } + + // Dev branches track a moving target; allow permanent deletion. + // Stable versions are immutable snapshots and are only soft-deleted here, so they can be + // recovered later by the maintainer or surfaced to administrators if needed. + $softDeleted = !$version->isDevelopment(); + $deletionTitle = null; + if ($softDeleted) { + // An admin who also maintains the package is acting on their own package — record it as + // a maintainer deletion. The dedicated admin-delete route is the explicit signal for + // "use admin powers here." + $reason = !$package->isMaintainer($user) && $this->isGranted(PackageActions::AdminDeleteVersion->value, $package) + ? VersionDeletionReason::DeletedByAdmin + : VersionDeletionReason::DeletedByMaintainer; + $repo->softDelete($version, $reason, null, $user); + $deletionTitle = $reason === VersionDeletionReason::DeletedByAdmin + ? 'Removed by admin on '.gmdate('Y-m-d H:i:s').' UTC' + : 'Deleted by maintainer on '.gmdate('Y-m-d H:i:s').' UTC'; + } else { + $repo->remove($version); + } + + $this->getEM()->flush(); + $this->getEM()->clear(); + + return new JsonResponse(['softDeleted' => $softDeleted, 'deletionTitle' => $deletionTitle]); + } + + #[Route(path: '/versions/{versionId}/admin-delete', name: 'admin_delete_version', requirements: ['versionId' => '[0-9]+'], methods: ['POST'])] + public function adminSoftDeleteVersionAction(Request $req, int $versionId, #[CurrentUser] User $user): Response + { + $repo = $this->getEM()->getRepository(Version::class); + try { + $version = $repo->getFullVersion($versionId); + } catch (NoResultException) { + return new Response('Version '.$versionId.' not found', 404); + } + + $this->denyAccessUnlessGranted(PackageActions::AdminDeleteVersion->value, $version->getPackage(), 'No permission to admin-delete versions'); + + if (!$this->isCsrfTokenValid('manage_versions', $req->request->getString('_token'))) { + throw new AccessDeniedException('Invalid CSRF token'); + } + + $reasonText = trim($req->request->getString('reason')); + if ($reasonText === '') { + $reasonText = null; + } + + $repo->softDelete($version, VersionDeletionReason::DeletedByAdmin, $reasonText, $user); + $this->getEM()->flush(); + $this->getEM()->clear(); + + $deletionTitle = 'Removed by admin on '.gmdate('Y-m-d H:i:s').' UTC' + .($reasonText !== null ? ': '.$reasonText : ''); + + return new JsonResponse(['softDeleted' => true, 'deletionTitle' => $deletionTitle]); + } + + #[Route(path: '/versions/{versionId}/admin-hide', name: 'admin_hide_version', requirements: ['versionId' => '[0-9]+'], methods: ['POST'])] + public function adminHideVersionAction(Request $req, int $versionId, #[CurrentUser] User $user): Response + { + $repo = $this->getEM()->getRepository(Version::class); + try { + $version = $repo->getFullVersion($versionId); + } catch (NoResultException) { + return new Response('Version '.$versionId.' not found', 404); + } + + $this->denyAccessUnlessGranted(PackageActions::HideVersion->value, $version->getPackage(), 'No permission to hide versions'); + + if (!$this->isCsrfTokenValid('manage_versions', $req->request->getString('_token'))) { + throw new AccessDeniedException('Invalid CSRF token'); + } + + $reasonText = trim($req->request->getString('reason')); + if ($reasonText === '') { + $reasonText = null; + } + + $repo->softDelete($version, VersionDeletionReason::Hidden, $reasonText, $user); + $this->getEM()->flush(); + $this->getEM()->clear(); + + $deletionTitle = 'Hidden by admin on '.gmdate('Y-m-d H:i:s').' UTC' + .($reasonText !== null ? ': '.$reasonText : ''); + + return new JsonResponse(['softDeleted' => true, 'deletionTitle' => $deletionTitle, 'deletionIcon' => 'glyphicon-eye-close']); + } + + #[Route(path: '/versions/{versionId}/recover', name: 'recover_version', requirements: ['versionId' => '[0-9]+'], methods: ['POST'])] + public function recoverPackageVersionAction(Request $req, int $versionId, #[CurrentUser] User $user): Response + { + $repo = $this->getEM()->getRepository(Version::class); + try { + $version = $repo->getFullVersion($versionId); + } catch (NoResultException) { + return new Response('Version '.$versionId.' not found', 404); + } + $package = $version->getPackage(); + + $this->denyAccessUnlessGranted(PackageActions::RecoverVersion->value, $package, 'No permission to recover versions'); + + if (!$this->isCsrfTokenValid('manage_versions', $req->request->getString('_token'))) { throw new AccessDeniedException('Invalid CSRF token'); } - $repo->remove($version); + // Per-reason recovery: admins can recover anything; maintainers can only recover what they + // (or the auto-missing flow) created. AdminDeleted rows stay locked unless an admin acts. + $reason = $version->getDeletionReason(); + if ($reason !== null && !$reason->isRecoverableByMaintainer() && !$this->isGranted(PackageActions::AdminDeleteVersion->value, $package)) { + throw new AccessDeniedException('This version was removed by an administrator and cannot be recovered here.'); + } + + $repo->recover($version, $user); $this->getEM()->flush(); $this->getEM()->clear(); @@ -845,7 +978,7 @@ public function updatePackageAction(Request $req, string $name, #[CurrentUser] U $update = $req->request->getBoolean('update', $req->query->getBoolean('update')); $autoUpdated = $req->request->get('autoUpdated', $req->query->get('autoUpdated')); - $updateEqualRefs = $req->request->getBoolean('updateAll', $req->query->getBoolean('updateAll')); + $updateSourceDistUrl = $req->request->getBoolean('updateAll', $req->query->getBoolean('updateAll')); $manualUpdate = $req->request->getBoolean('manualUpdate', $req->query->getBoolean('manualUpdate')); // check that a user is logged in to trigger an update on a package they don't own at least to avoid abuse @@ -859,7 +992,7 @@ public function updatePackageAction(Request $req, string $name, #[CurrentUser] U // do not let non-maintainers execute update with those flags if (!$canUpdatePackage) { $autoUpdated = null; - $updateEqualRefs = false; + $updateSourceDistUrl = false; $manualUpdate = false; } @@ -869,7 +1002,7 @@ public function updatePackageAction(Request $req, string $name, #[CurrentUser] U } if ($update) { - $job = $this->scheduler->scheduleUpdate($package, 'button/api', $updateEqualRefs, false, null, $manualUpdate); + $job = $this->scheduler->scheduleUpdate($package, 'button/api', $updateSourceDistUrl, false, null, $manualUpdate); return new JsonResponse(['status' => 'success', 'job' => $job->getId()], 202); } diff --git a/src/Controller/UserController.php b/src/Controller/UserController.php index 710675f89..fb63a2be2 100644 --- a/src/Controller/UserController.php +++ b/src/Controller/UserController.php @@ -13,6 +13,7 @@ namespace App\Controller; use App\Attribute\VarName; +use App\Audit\VersionDeletionReason; use App\Entity\Package; use App\Entity\TemporaryTwoFactorUser; use App\Entity\User; @@ -89,7 +90,7 @@ public function triggerGitHubSyncAction(#[CurrentUser] User $user): RedirectResp } #[Route(path: '/spammers/{name}/', name: 'mark_spammer', methods: ['POST'])] - public function markSpammerAction(Request $req, #[VarName('name')] User $user): RedirectResponse + public function markSpammerAction(Request $req, #[VarName('name')] User $user, #[CurrentUser] User $adminUser): RedirectResponse { if (!$this->isGranted('ROLE_ANTISPAM')) { throw $this->createAccessDeniedException('This user can not mark others as spammers'); @@ -123,7 +124,10 @@ public function markSpammerAction(Request $req, #[VarName('name')] User $user): foreach ($packages as $package) { foreach ($package->getVersions() as $version) { - $versionRepo->remove($version); + if ($version->isSoftDeleted()) { + continue; + } + $versionRepo->softDelete($version, VersionDeletionReason::Hidden, 'spam', $adminUser); } $this->providerManager->deletePackage($package); diff --git a/src/DataFixtures/PackageFixtures.php b/src/DataFixtures/PackageFixtures.php index b564f9ef3..b8dc24743 100644 --- a/src/DataFixtures/PackageFixtures.php +++ b/src/DataFixtures/PackageFixtures.php @@ -109,7 +109,7 @@ private function updatePackage(int $id): void { $job = new Job('FAKE_ID', 'FAKE_TYPE', [ 'id' => $id, - 'update_equal_refs' => false, + 'update_source_dist_url' => false, 'delete_before' => false, 'force_dump' => false, 'source' => 'fixtures', diff --git a/src/Entity/AuditRecord.php b/src/Entity/AuditRecord.php index 991afd28d..b205e5344 100644 --- a/src/Entity/AuditRecord.php +++ b/src/Entity/AuditRecord.php @@ -15,6 +15,7 @@ use App\Audit\AbandonmentReason; use App\Audit\AuditRecordType; use App\Audit\UserRegistrationMethod; +use App\Audit\VersionDeletionReason; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Uid\Ulid; @@ -111,6 +112,42 @@ public static function versionDeleted(Version $version, ?User $actor): self return new self(AuditRecordType::VersionDeleted, ['name' => $package->getName(), 'version' => $version->getVersion(), 'actor' => self::getUserData($actor, 'automation')], $actor?->getId(), $package->getVendor(), $package->getId()); } + public static function versionSoftDeleted(Version $version, VersionDeletionReason $reason, ?string $reasonText, ?User $actor): self + { + $package = $version->getPackage(); + + return new self( + AuditRecordType::VersionSoftDeleted, + ['name' => $package->getName(), 'version' => $version->getVersion(), 'reason' => $reason->value, 'reasonText' => $reasonText, 'actor' => self::getUserData($actor, 'automation')], + $actor?->getId(), + $package->getVendor(), + $package->getId() + ); + } + + public static function versionRecovered(Version $version, VersionDeletionReason $previousReason, ?User $actor): self + { + $package = $version->getPackage(); + + return new self( + AuditRecordType::VersionRecovered, + ['name' => $package->getName(), 'version' => $version->getVersion(), 'previousReason' => $previousReason->value, 'actor' => self::getUserData($actor, 'automation')], + $actor?->getId(), + $package->getVendor(), + $package->getId() + ); + } + + public static function versionReferenceChangeBlocked(Package $package, string $prettyVersion, ?string $oldRef, string $newRef): self + { + return new self( + AuditRecordType::VersionReferenceChangeBlocked, + ['name' => $package->getName(), 'version' => $prettyVersion, 'ref_from' => $oldRef, 'ref_to' => $newRef], + vendor: $package->getVendor(), + packageId: $package->getId() + ); + } + /** * @param VersionArray $metadata */ diff --git a/src/Entity/Version.php b/src/Entity/Version.php index 91f6bcbba..fd6c0b4a7 100644 --- a/src/Entity/Version.php +++ b/src/Entity/Version.php @@ -12,6 +12,7 @@ namespace App\Entity; +use App\Audit\VersionDeletionReason; use Composer\Package\Version\VersionParser; use Composer\Pcre\Preg; use Doctrine\Common\Collections\ArrayCollection; @@ -59,6 +60,7 @@ #[ORM\UniqueConstraint(name: 'pkg_ver_idx', columns: ['package_id', 'normalizedVersion'])] #[ORM\Index(name: 'release_idx', columns: ['releasedAt'])] #[ORM\Index(name: 'is_devel_idx', columns: ['development'])] +#[ORM\Index(name: 'softdel_reason_idx', columns: ['softDeletedAt', 'deletionReason'])] class Version { #[ORM\Id] @@ -219,6 +221,15 @@ class Version #[ORM\Column(type: 'datetime_immutable', nullable: true)] private ?\DateTimeImmutable $softDeletedAt = null; + #[ORM\Column(length: 32, nullable: true, enumType: VersionDeletionReason::class)] + private ?VersionDeletionReason $deletionReason = null; + + #[ORM\Column(type: 'text', length: 65535, nullable: true)] + private ?string $deletionReasonText = null; + + #[ORM\Column(length: 255, nullable: true)] + private ?string $lastBlockedReference = null; + #[ORM\Column(type: 'datetime_immutable')] private \DateTimeImmutable $updatedAt; @@ -678,6 +689,59 @@ public function getSoftDeletedAt(): ?\DateTimeImmutable return $this->softDeletedAt; } + public function isSoftDeleted(): bool + { + return $this->softDeletedAt !== null; + } + + public function setDeletionReason(?VersionDeletionReason $reason): void + { + $this->deletionReason = $reason; + } + + public function getDeletionReason(): ?VersionDeletionReason + { + return $this->deletionReason; + } + + public function setDeletionReasonText(?string $text): void + { + $this->deletionReasonText = $text; + } + + public function getDeletionReasonText(): ?string + { + return $this->deletionReasonText; + } + + public function setLastBlockedReference(?string $ref): void + { + $this->lastBlockedReference = $ref; + } + + public function getLastBlockedReference(): ?string + { + return $this->lastBlockedReference; + } + + /** + * Effective reference used for immutability identity: source.reference if present, + * else dist.reference, else null (no usable identity). + */ + public function getEffectiveReference(): ?string + { + $sourceRef = $this->source['reference'] ?? null; + if (\is_string($sourceRef) && $sourceRef !== '') { + return $sourceRef; + } + $distRef = $this->dist['reference'] ?? null; + if (\is_string($distRef) && $distRef !== '') { + return $distRef; + } + + return null; + } + /** * @return array */ diff --git a/src/Entity/VersionRepository.php b/src/Entity/VersionRepository.php index bd1b44814..1880578f3 100644 --- a/src/Entity/VersionRepository.php +++ b/src/Entity/VersionRepository.php @@ -12,7 +12,9 @@ namespace App\Entity; +use App\Audit\VersionDeletionReason; use App\Model\VersionIdCache; +use App\Service\Scheduler; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\DBAL\ArrayParameterType; use Doctrine\DBAL\Cache\QueryCacheProfile; @@ -36,6 +38,7 @@ public function __construct( private Client $redisCache, private VersionIdCache $versionIdCache, private readonly Security $security, + private readonly Scheduler $scheduler, ) { parent::__construct($registry, Version::class); } @@ -77,6 +80,74 @@ public function remove(Version $version, bool $createAuditRecord = true): void } } + /** + * Soft-delete a version with a reason. Schedules a fresh Updater run so dependents/suggesters + * and the V2 dump get recomputed without the version. + */ + public function softDelete(Version $version, VersionDeletionReason $reason, ?string $reasonText, ?User $actor): void + { + $em = $this->getEntityManager(); + $version->setSoftDeletedAt(new \DateTimeImmutable()); + $version->setDeletionReason($reason); + $version->setDeletionReasonText($reasonText); + $em->persist($version); + + $em->persist(AuditRecord::versionSoftDeleted($version, $reason, $reasonText, $actor)); + + $this->scheduler->scheduleUpdate($version->getPackage(), 'version_soft_delete'); + } + + /** + * Bulk-recover every Hidden version of the given package. Used by the unfreeze flow so an + * admin reversing a spam decision restores all the hidden versions in one go without N + * scheduler dispatches. Emits one audit row per recovered version. + */ + public function recoverHiddenVersionsForPackage(Package $package, ?User $actor): int + { + $em = $this->getEntityManager(); + /** @var Version[] $hidden */ + $hidden = $this->createQueryBuilder('v') + ->where('v.package = :package') + ->andWhere('v.deletionReason = :reason') + ->setParameter('package', $package) + ->setParameter('reason', VersionDeletionReason::Hidden) + ->getQuery()->getResult(); + + if (\count($hidden) === 0) { + return 0; + } + + foreach ($hidden as $version) { + $version->setSoftDeletedAt(null); + $version->setDeletionReason(null); + $version->setDeletionReasonText(null); + $em->persist($version); + $em->persist(AuditRecord::versionRecovered($version, VersionDeletionReason::Hidden, $actor)); + } + + $this->scheduler->scheduleUpdate($package, 'unfreeze_recover_hidden'); + + return \count($hidden); + } + + /** + * Clear a soft-delete marking. Schedules a fresh Updater run for the same reasons as softDelete(). + */ + public function recover(Version $version, ?User $actor): void + { + $previousReason = $version->getDeletionReason() ?? VersionDeletionReason::AutoDeletedMissing; + + $em = $this->getEntityManager(); + $version->setSoftDeletedAt(null); + $version->setDeletionReason(null); + $version->setDeletionReasonText(null); + $em->persist($version); + + $em->persist(AuditRecord::versionRecovered($version, $previousReason, $actor)); + + $this->scheduler->scheduleUpdate($version->getPackage(), 'version_recover'); + } + /** * @param Version[] $versions * @@ -181,7 +252,7 @@ public function getVersionData(array $versionIds): array public function getVersionMetadataForUpdate(Package $package): array { $rows = $this->getEntityManager()->getConnection()->fetchAllAssociative( - 'SELECT id, version, normalizedVersion, source, softDeletedAt, defaultBranch FROM package_version v WHERE v.package_id = :id', + 'SELECT id, version, normalizedVersion, development, source, dist, softDeletedAt, deletionReason, lastBlockedReference, defaultBranch FROM package_version v WHERE v.package_id = :id', ['id' => $package->getId()] ); @@ -190,6 +261,9 @@ public function getVersionMetadataForUpdate(Package $package): array if ($row['source']) { $row['source'] = json_decode($row['source'], true); } + if ($row['dist']) { + $row['dist'] = json_decode($row['dist'], true); + } $versions[strtolower($row['normalizedVersion'])] = $row; } diff --git a/src/Model/PackageManager.php b/src/Model/PackageManager.php index 80d5bda0e..6d2f19967 100644 --- a/src/Model/PackageManager.php +++ b/src/Model/PackageManager.php @@ -22,6 +22,7 @@ use App\Entity\PhpStat; use App\Entity\User; use App\Entity\Version; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use App\Service\CdnClient; use App\Service\GitHubUserMigrationWorker; use Composer\Pcre\Preg; @@ -56,6 +57,7 @@ public function __construct( private Client $redis, private VersionIdCache $versionIdCache, private readonly CdnClient $cdnClient, + private readonly UrlGeneratorInterface $urlGenerator, ) { } @@ -215,6 +217,52 @@ public function notifyUpdateFailure(Package $package, \Exception $e, ?string $de return true; } + /** + * Notify maintainers that an upstream re-tag was blocked because the affected stable version is immutable. + * Emits one message per (version, attempted reference); the Updater dedupes via Version::$lastBlockedReference. + */ + public function notifyVersionReferenceChangeBlocked(Package $package, string $prettyVersion, ?string $oldRef, string $newRef): bool + { + $recipients = []; + foreach ($package->getMaintainers() as $maintainer) { + $mail = $maintainer->getEmail(); + if ($mail && $maintainer->isNotifiableForFailures()) { + $recipients[$mail] = new Address($mail, $maintainer->getUsername()); + } + } + + if (!$recipients) { + return false; + } + + $body = $this->twig->render('email/version_reference_change_blocked.txt.twig', [ + 'package' => $package, + 'version' => $prettyVersion, + 'oldRef' => $oldRef, + 'newRef' => $newRef, + 'packageUrl' => $this->urlGenerator->generate('view_package', ['name' => $package->getName()], UrlGeneratorInterface::ABSOLUTE_URL), + 'docUrl' => $this->urlGenerator->generate('about_version_immutability', [], UrlGeneratorInterface::ABSOLUTE_URL), + ]); + + $message = new Email() + ->subject($package->getName().': re-tag of '.$prettyVersion.' blocked') + ->from(new Address($this->options['from'], $this->options['fromName'])) + ->to(...array_values($recipients)) + ->text($body) + ; + $message->getHeaders()->addTextHeader('X-Auto-Response-Suppress', 'OOF, DR, RN, NRN, AutoReply'); + + try { + $this->mailer->send($message); + } catch (\Symfony\Component\Mailer\Exception\TransportExceptionInterface $e) { + $this->logger->error('['.$e::class.'] '.$e->getMessage()); + + return false; + } + + return true; + } + public function notifyNewMaintainer(User $user, Package $package): bool { $body = $this->twig->render('email/maintainer_added.txt.twig', [ diff --git a/src/Package/Updater.php b/src/Package/Updater.php index 4ceb4ac73..f74b0fe08 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -13,6 +13,8 @@ namespace App\Package; use App\Audit\AbandonmentReason; +use App\Audit\VersionDeletionReason; +use App\Entity\AuditRecord; use App\Entity\ConflictLink; use App\Entity\Dependent; use App\Entity\DevRequireLink; @@ -30,6 +32,7 @@ use App\Event\VersionReferenceChangedEvent; use App\HtmlSanitizer\ReadmeImageSanitizer; use App\HtmlSanitizer\ReadmeLinkSanitizer; +use App\Model\PackageManager; use App\Model\ProviderManager; use App\Model\VersionIdCache; use App\Service\VersionCache; @@ -49,6 +52,7 @@ use Composer\Util\HttpDownloader; use Doctrine\DBAL\ArrayParameterType; use Doctrine\Persistence\ManagerRegistry; +use Psr\Log\LoggerInterface; use Symfony\Component\HtmlSanitizer\HtmlSanitizer; use Symfony\Component\HtmlSanitizer\HtmlSanitizerAction; use Symfony\Component\HtmlSanitizer\HtmlSanitizerConfig; @@ -76,6 +80,11 @@ public function __construct( public function __construct( public int $id, public string $version, + /** + * True when the skip is because the row is intentionally soft-deleted (admin or maintainer). + * The dependent/suggester source selection skips these so a pulled version doesn't poison metadata. + */ + public bool $softDeleted = false, ) { } } @@ -87,7 +96,12 @@ class Updater { use \App\Util\DoctrineTrait; - public const UPDATE_EQUAL_REFS = 1; + /** + * Roll out new source/dist URLs across existing versions without changing references. + * Stable versions only accept the rewrite when every check in updateSourceDistUrl() passes + * (matching commit-hash refs, driver-confirmed dist URL, etc.); otherwise that version is skipped. + */ + public const UPDATE_SOURCE_DIST_URL = 1; public const DELETE_BEFORE = 2; public const FORCE_DUMP = 4; @@ -132,10 +146,28 @@ public function __construct( private string $mailFromEmail, private UrlGeneratorInterface $urlGenerator, private EventDispatcherInterface $eventDispatcher, + private PackageManager $packageManager, + private LoggerInterface $logger, ) { ErrorHandler::register(); } + /** + * Identity used for immutability comparisons: source.reference if present and non-empty, + * else dist.reference, else null. A version with no effective reference cannot be imported. + */ + public static function computeEffectiveReference(?string $sourceRef, ?string $distRef): ?string + { + if (\is_string($sourceRef) && $sourceRef !== '') { + return $sourceRef; + } + if (\is_string($distRef) && $distRef !== '') { + return $distRef; + } + + return null; + } + public function __destruct() { restore_error_handler(); @@ -245,8 +277,14 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep $versionRepository = $this->doctrine->getRepository(Version::class); if ($flags & self::DELETE_BEFORE) { + // Stable, published versions are immutable historical records and must never be + // hard-deleted. DELETE_BEFORE wipes the dev-branch trackers and lets the normal update + // path reconcile stable rows via the immutability gate (matching refs are skipped, + // diverging refs are blocked + audited). foreach ($package->getVersions() as $version) { - $versionRepository->remove($version); + if ($version->isDevelopment()) { + $versionRepository->remove($version); + } } $em->flush(); @@ -277,7 +315,13 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep $processedVersions[strtolower($version->getVersion())] = $version; $result = $this->updateInformation($io, $versionRepository, $package, $existingVersions, $version, $flags, $rootIdentifier, $driver); + if ($result === null) { + // version was rejected outright (no usable reference and not in DB) + continue; + } + $versionId = false; + $versionSoftDeleted = false; if ($result instanceof VersionUpdatedResult) { foreach ($result->events as $event) { $this->eventDispatcher->dispatch($event); @@ -292,14 +336,13 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep $versionId = $result->entity->getId(); } else { $idsToMarkUpdated[] = $result->id; + $versionSoftDeleted = $result->softDeleted; } - // use the first version which should be the highest stable version by default - if (null === $dependentSuggesterSource) { + // pick the default branch when present, otherwise the first non-soft-deleted version + if ($version->isDefaultBranch() && !$versionSoftDeleted) { $dependentSuggesterSource = $versionId; - } - // if default branch is present however we prefer that as the canonical source of dependent/suggester - if ($version->isDefaultBranch()) { + } elseif (null === $dependentSuggesterSource && !$versionSoftDeleted) { $dependentSuggesterSource = $versionId; } @@ -311,20 +354,39 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep $this->doctrine->getRepository(Dependent::class)->updateDependentSuggesters($package->getId(), $dependentSuggesterSource); } - // make sure versions that are still present but did not update are not pruned + // auto-recover versions still present upstream that had been auto-soft-deleted as missing. + // Admin/maintainer-pulled rows (deletionReason in (admin, maintainer)) stay soft-deleted. $em->getConnection()->executeStatement( - 'UPDATE package_version SET updatedAt = :now, softDeletedAt = NULL WHERE id IN (:ids) AND softDeletedAt IS NOT NULL', - ['now' => date('Y-m-d H:i:s'), 'ids' => $idsToMarkUpdated], + 'UPDATE package_version + SET updatedAt = :now, softDeletedAt = NULL, deletionReason = NULL, deletionReasonText = NULL + WHERE id IN (:ids) + AND softDeletedAt IS NOT NULL + AND (deletionReason IS NULL OR deletionReason = :autoReason)', + ['now' => date('Y-m-d H:i:s'), 'autoReason' => VersionDeletionReason::AutoDeletedMissing->value, 'ids' => $idsToMarkUpdated], ['ids' => ArrayParameterType::INTEGER] ); - // remove outdated versions + // remove or soft-mark versions that disappeared from upstream foreach ($existingVersions as $version) { + $existingReason = $version['deletionReason'] ?? null; + $isAutoOrEmpty = $existingReason === null || $existingReason === VersionDeletionReason::AutoDeletedMissing->value; + $isDev = (bool) $version['development']; + + // Intentionally-pulled versions (admin/maintainer/hidden): leave them as they are. + if (!$isAutoOrEmpty) { + continue; + } + + // Dev rows are branch trackers and may be hard-purged after a 1-day grace period + // (or immediately if they're legacy v1-normalized dev-master/trunk/default rows that + // got re-created under a non-normalized name). Stable rows are immutable historical + // entries and stay soft-deleted forever — see VersionDeletionReason. if ( - // soft-deleted versions are really purged after a day - (null !== $version['softDeletedAt'] && new \DateTime($version['softDeletedAt']) < $deleteDate) - // remove v1 normalized versions of dev-master/trunk/default immediately as they have been recreated as dev-master/trunk/default in a non-normalized way - || ($version['normalizedVersion'] === '9999999-dev') + $isDev + && ( + (null !== $version['softDeletedAt'] && new \DateTime($version['softDeletedAt']) < $deleteDate) + || ($version['normalizedVersion'] === '9999999-dev') + ) ) { $versionEntity = $versionRepository->find($version['id']); if (null !== $versionEntity) { @@ -333,12 +395,14 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep continue; } - // set it to be soft-deleted so next update that occurs after deleteDate (1day) if the - // version is still missing it will be really removed - $em->getConnection()->executeStatement( - 'UPDATE package_version SET softDeletedAt = :now WHERE id = :id', - ['now' => date('Y-m-d H:i:s'), 'id' => $version['id']] - ); + // First-time soft-mark only. Re-stamping softDeletedAt on every crawl would reset the + // 1-day grace window for dev rows and is meaningless noise for stable rows. + if (null === $version['softDeletedAt']) { + $em->getConnection()->executeStatement( + 'UPDATE package_version SET softDeletedAt = :now, deletionReason = :reason WHERE id = :id', + ['now' => date('Y-m-d H:i:s'), 'reason' => VersionDeletionReason::AutoDeletedMissing->value, 'id' => $version['id']] + ); + } } if (null !== ($match = $package->getGitHubComponents())) { @@ -381,16 +445,153 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep } /** - * Keys info: + * Apply the immutability rule to an existing stable version. Returns the skip result; never mutates + * the version's source/dist/metadata. The only DB writes performed here are to lastBlockedReference + * (set or clear) and the audit/email side-effects when a new attempted ref is detected. * - * - updated (whether the version was updated or needs to be marked as updated) - * - id (version id, can be null for newly created versions) - * - version (normalized version from the composer package) - * - object (Version instance if it was updated) + * @param array{id: int, version: string, normalizedVersion: string, development: int, source: array{type: string|null, url: string|null, reference: string|null}|null, dist: array{type: string|null, url: string|null, reference: string|null, shasum: string|null}|null, softDeletedAt: string|null, deletionReason: string|null, lastBlockedReference: string|null, defaultBranch: int} $existingVersion + */ + private function applyStableImmutabilityGate(IOInterface $io, \Doctrine\ORM\EntityManagerInterface $em, Package $package, array $existingVersion, CompletePackageInterface $data, int $flags, VcsDriverInterface $driver, ?string $newEffectiveRef): VersionSkippedResult + { + $normVersion = $data->getVersion(); + $prettyVersion = $data->getPrettyVersion(); + $oldEffectiveRef = self::computeEffectiveReference($existingVersion['source']['reference'] ?? null, $existingVersion['dist']['reference'] ?? null); + $lastBlocked = $existingVersion['lastBlockedReference']; + + // Incoming data with no usable reference: refuse to mutate. Don't treat as a block event + // (broken driver output, not an upstream re-tag). + if ($newEffectiveRef === null) { + $io->writeError('Skipping update of '.$prettyVersion.': incoming data has no usable source/dist reference.'); + + return new VersionSkippedResult(id: $existingVersion['id'], version: strtolower($normVersion)); + } + + if ($oldEffectiveRef === $newEffectiveRef) { + // Divergence resolved: clear lastBlockedReference so the UI badge disappears and a future + // re-divergence to this same ref correctly re-fires the audit/email path. + if ($lastBlocked !== null) { + $em->getConnection()->executeStatement( + 'UPDATE package_version SET lastBlockedReference = NULL WHERE id = :id', + ['id' => $existingVersion['id']] + ); + } + + if ($flags & self::UPDATE_SOURCE_DIST_URL) { + $this->applySourceDistUrlRewrite($io, $em, $existingVersion, $data, $driver); + } + + return new VersionSkippedResult(id: $existingVersion['id'], version: strtolower($normVersion)); + } + + // Reference differs: this is an immutability violation. Dedupe by the attempted ref so the + // maintainer log and audit table don't accumulate duplicate entries on every crawl. + if ($lastBlocked === $newEffectiveRef) { + return new VersionSkippedResult(id: $existingVersion['id'], version: strtolower($normVersion)); + } + + $io->writeError('Refusing to update stable version '.$prettyVersion.': reference changed (was '.($oldEffectiveRef ?? '').', now '.$newEffectiveRef.'). Stable versions are immutable; tag a new version to publish changes.'); + + // Audit + email + remember the attempted ref. + $em->persist(AuditRecord::versionReferenceChangeBlocked($package, $prettyVersion, $oldEffectiveRef, $newEffectiveRef)); + $em->getConnection()->executeStatement( + 'UPDATE package_version SET lastBlockedReference = :ref WHERE id = :id', + ['ref' => $newEffectiveRef, 'id' => $existingVersion['id']] + ); + $this->packageManager->notifyVersionReferenceChangeBlocked($package, $prettyVersion, $oldEffectiveRef, $newEffectiveRef); + + return new VersionSkippedResult(id: $existingVersion['id'], version: strtolower($normVersion)); + } + + /** + * Strict source/dist URL rewrite for stable versions under UPDATE_SOURCE_DIST_URL. + * Rewrites only source.url and dist.url; leaves references, shasum, and every other field alone. + * Skips the row (with a specific warning) if any of the safety checks fail. + * + * @param array{id: int, source: array{type: string|null, url: string|null, reference: string|null}|null, dist: array{type: string|null, url: string|null, reference: string|null, shasum: string|null}|null} $existingVersion + */ + private function applySourceDistUrlRewrite(IOInterface $io, \Doctrine\ORM\EntityManagerInterface $em, array $existingVersion, CompletePackageInterface $data, VcsDriverInterface $driver): void + { + $prettyVersion = $data->getPrettyVersion(); + $skip = function (string $reason) use ($io, $prettyVersion, $data, $existingVersion): void { + $this->logger->error('Skipped URL update for '.$data->getName(), ['reason' => $reason, 'prettyVersion' => $prettyVersion, 'old' => $existingVersion, 'new' => $data]); + $io->writeError('Skipping URL update on '.$prettyVersion.': '.$reason.'.'); + }; + + $oldSource = $existingVersion['source'] ?? null; + $oldDist = $existingVersion['dist'] ?? null; + $oldSourceRef = $oldSource['reference'] ?? null; + $oldDistRef = $oldDist['reference'] ?? null; + $newSourceRef = $data->getSourceReference(); + $newDistRef = $data->getDistReference(); + $newSourceUrl = $data->getSourceUrl(); + $newDistUrl = $data->getDistUrl(); + + if (!\is_string($oldSourceRef) || $oldSourceRef === '' || !\is_string($oldDistRef) || $oldDistRef === '') { + $skip('existing source/dist reference is missing'); + + return; + } + if (!\is_string($newSourceRef) || $newSourceRef === '' || !\is_string($newDistRef) || $newDistRef === '') { + $skip('new source/dist reference is missing'); + + return; + } + if (!Preg::isMatch('{^[a-f0-9]{40,}$}', $oldSourceRef) || !Preg::isMatch('{^[a-f0-9]{40,}$}', $oldDistRef)) { + $skip('reference is not a 40+ char commit hash'); + + return; + } + if ($oldSourceRef !== $newSourceRef || $oldDistRef !== $newDistRef) { + $skip('source/dist reference hashes differ between stored and new data'); + + return; + } + if (!\is_string($newSourceUrl) || $newSourceUrl === '' || !\is_string($newDistUrl) || $newDistUrl === '') { + $skip('new source/dist URL is missing'); + + return; + } + + // Authoritative check: ask the VCS driver what dist URL belongs to this reference. + try { + $driverDist = $driver->getDist($newDistRef); + } catch (\Throwable $e) { + $skip('driver getDist() threw: '.$e->getMessage()); + + return; + } + if (!is_array($driverDist) || ($driverDist['url'] ?? null) !== $newDistUrl) { + $skip('driver-confirmed dist URL does not match incoming dist URL'); + + return; + } + + $newSource = ($oldSource ?? []) + []; + $newSource['url'] = $newSourceUrl; + $newDist = ($oldDist ?? []) + []; + $newDist['url'] = $newDistUrl; + + $em->getConnection()->executeStatement( + 'UPDATE package_version SET source = :source, dist = :dist WHERE id = :id', + [ + 'source' => json_encode($newSource, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE), + 'dist' => json_encode($newDist, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE), + 'id' => $existingVersion['id'], + ] + ); + } + + /** + * Decide what to do with one upstream version vs the DB state, and apply the change. + * + * Returns: + * - null when the version is rejected outright (e.g. brand-new but no usable reference) + * - VersionSkippedResult when the existing row is left alone (immutable stable, soft-deleted, etc.) + * - VersionUpdatedResult when the row is created or mutated * * @param ExistingVersionsForUpdate $existingVersions */ - private function updateInformation(IOInterface $io, VersionRepository $versionRepo, Package $package, array $existingVersions, CompletePackageInterface $data, int $flags, string $rootIdentifier, VcsDriverInterface $driver): VersionSkippedResult|VersionUpdatedResult + private function updateInformation(IOInterface $io, VersionRepository $versionRepo, Package $package, array $existingVersions, CompletePackageInterface $data, int $flags, string $rootIdentifier, VcsDriverInterface $driver): VersionSkippedResult|VersionUpdatedResult|null { $em = $this->getEM(); $version = new Version(); @@ -398,18 +599,39 @@ private function updateInformation(IOInterface $io, VersionRepository $versionRe $postUpdateEvents = []; $normVersion = $data->getVersion(); + $newSourceRef = $data->getSourceReference(); + $newDistRef = $data->getDistReference(); + $newEffectiveRef = self::computeEffectiveReference($newSourceRef, $newDistRef); $existingVersion = $existingVersions[strtolower($normVersion)] ?? null; + + if ($existingVersion) { + // Intentionally-pulled versions are never recreated or modified by the Updater + $existingReason = isset($existingVersion['deletionReason']) ? VersionDeletionReason::tryFrom((string) $existingVersion['deletionReason']) : null; + if ($existingReason !== null && $existingReason !== VersionDeletionReason::AutoDeletedMissing) { + return new VersionSkippedResult( + id: $existingVersion['id'], + version: strtolower($normVersion), + softDeleted: true, + ); + } + + // Stable-version immutability gate: any existing stable version is frozen. + if (!$data->isDev()) { + return $this->applyStableImmutabilityGate($io, $em, $package, $existingVersion, $data, $flags, $driver, $newEffectiveRef); + } + } elseif ($newEffectiveRef === null) { + // Brand-new version with no usable identity: refuse to create. + $io->writeError('Skipping '.$data->getPrettyVersion().': no usable source/dist reference.'); + + return null; + } + if ($existingVersion) { - $source = $existingVersion['source']; + // Dev-version flow (existing version). Update on reference change, or abandoned flags & default-branch status changes. if ( - // update if the source reference has changed (re-tag or new commit on branch) - ($source['reference'] ?? null) !== $data->getSourceReference() - // or the source has some corrupted github private url - || (isset($source['url']) && \is_string($source['url']) && Preg::isMatch('{^git@github.com:.*?\.git$}', $source['url'])) - // or if the right flag is set - || ($flags & self::UPDATE_EQUAL_REFS) - // or if the package must be marked abandoned from composer.json + ($existingVersion['source']['reference'] ?? null) !== $newSourceRef + || ($flags & self::UPDATE_SOURCE_DIST_URL) || ($data->isAbandoned() && !$package->isAbandoned()) || ($data->isAbandoned() && $data->getReplacementPackage() !== $package->getReplacementPackage()) ) { @@ -418,10 +640,8 @@ private function updateInformation(IOInterface $io, VersionRepository $versionRe throw new \LogicException('At this point a version should always be found'); } $versionId = $version->getId(); - } elseif ( + } elseif ($data->isDefaultBranch() !== (bool) $existingVersion['defaultBranch']) { // if the version default branch state has changed we update just that - $data->isDefaultBranch() !== (bool) $existingVersion['defaultBranch'] - ) { $version = $versionRepo->find($existingVersion['id']); if (null === $version) { throw new \LogicException('At this point a version should always be found'); @@ -478,6 +698,8 @@ private function updateInformation(IOInterface $io, VersionRepository $versionRe $version->setPackage($package); $version->setUpdatedAt(new \DateTimeImmutable()); $version->setSoftDeletedAt(null); + $version->setDeletionReason(null); + $version->setDeletionReasonText(null); $version->setReleasedAt($data->getReleaseDate() === null ? null : \DateTimeImmutable::createFromInterface($data->getReleaseDate())); if ($data->getSourceType() && !in_array($data->getSourceType(), ['perforce', 'fossil'], true)) { // null or '' here explicitly means no source and will be nulled, do not change this behavior diff --git a/src/Package/V2Dumper.php b/src/Package/V2Dumper.php index eda8030c6..c622103e3 100644 --- a/src/Package/V2Dumper.php +++ b/src/Package/V2Dumper.php @@ -294,7 +294,12 @@ private function dumpPackageToV2File(string $dir, Package $package, array $versi $name = strtolower($package->getName()); $forceDump = $package->getDumpedAtV2() === null; - $versions = $package->getVersions()->toArray(); + // Soft-deleted versions stay in the DB so the UI can list them grayed out, but they must not + // appear in metadata — Composer should not be able to resolve a pulled version. + $versions = array_filter( + $package->getVersions()->toArray(), + static fn (Version $v): bool => !$v->isSoftDeleted() + ); usort($versions, Package::sortVersions(...)); $tags = []; diff --git a/src/Security/Voter/PackageActions.php b/src/Security/Voter/PackageActions.php index f21ea67d0..51c3deb2a 100644 --- a/src/Security/Voter/PackageActions.php +++ b/src/Security/Voter/PackageActions.php @@ -21,5 +21,8 @@ enum PackageActions: string case Abandon = 'abandon'; case Delete = 'delete'; case DeleteVersion = 'delete_version'; + case RecoverVersion = 'recover_version'; + case AdminDeleteVersion = 'admin_delete_version'; + case HideVersion = 'hide_version'; case Update = 'update'; } diff --git a/src/Security/Voter/PackageVoter.php b/src/Security/Voter/PackageVoter.php index d970b9fed..412878b34 100644 --- a/src/Security/Voter/PackageVoter.php +++ b/src/Security/Voter/PackageVoter.php @@ -52,7 +52,9 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter return match (PackageActions::from($attribute)) { PackageActions::Abandon => $this->canEdit($package, $user), PackageActions::Delete => $this->canDelete($package, $user), - PackageActions::DeleteVersion => $this->canDeleteVersion($package, $user), + PackageActions::DeleteVersion, PackageActions::RecoverVersion => $this->canDeleteVersion($package, $user), + PackageActions::AdminDeleteVersion, + PackageActions::HideVersion => $this->canAdministerVersion(), PackageActions::Edit => $this->canEdit($package, $user), PackageActions::AddMaintainer, PackageActions::TransferPackage => $this->canAddMaintainers($package, $user), PackageActions::RemoveMaintainer => $this->canRemoveMaintainers($package, $user), @@ -60,6 +62,16 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter }; } + /** + * Privileged version-management actions reserved for administrators: deleting with a public + * reason and hiding from non-maintainers. The role check is the current implementation detail; + * call sites should depend on this voter, not on the role directly. + */ + private function canAdministerVersion(): bool + { + return $this->security->isGranted('ROLE_DELETE_PACKAGES'); + } + private function canDelete(Package $package, User $user): bool { if ($this->security->isGranted('ROLE_DELETE_PACKAGES')) { diff --git a/src/Service/Scheduler.php b/src/Service/Scheduler.php index 4b675ad99..412612595 100644 --- a/src/Service/Scheduler.php +++ b/src/Service/Scheduler.php @@ -32,13 +32,13 @@ public function __construct( /** * @return Job */ - public function scheduleUpdate(Package|int $packageOrId, string $source, bool $updateEqualRefs = false, bool $deleteBefore = false, ?\DateTimeImmutable $executeAfter = null, bool $forceDump = false): Job + public function scheduleUpdate(Package|int $packageOrId, string $source, bool $updateSourceDistUrl = false, bool $deleteBefore = false, ?\DateTimeImmutable $executeAfter = null, bool $forceDump = false): Job { if ($packageOrId instanceof Package) { $packageOrId = $packageOrId->getId(); } - $pendingJobId = $this->getPendingUpdateJob($packageOrId, $updateEqualRefs, $deleteBefore); + $pendingJobId = $this->getPendingUpdateJob($packageOrId, $updateSourceDistUrl, $deleteBefore); if ($pendingJobId && ($pendingJob = $this->getEM()->getRepository(Job::class)->findOneBy(['id' => $pendingJobId])) !== null) { // pending job will execute before the one we are trying to schedule so skip scheduling if ( @@ -59,7 +59,7 @@ public function scheduleUpdate(Package|int $packageOrId, string $source, bool $u $this->getEM()->flush(); } - return $this->createJob('package:updates', ['id' => $packageOrId, 'update_equal_refs' => $updateEqualRefs, 'delete_before' => $deleteBefore, 'force_dump' => $forceDump, 'source' => $source], $packageOrId, $executeAfter); + return $this->createJob('package:updates', ['id' => $packageOrId, 'update_source_dist_url' => $updateSourceDistUrl, 'delete_before' => $deleteBefore, 'force_dump' => $forceDump, 'source' => $source], $packageOrId, $executeAfter); } /** @@ -86,7 +86,7 @@ public function scheduleFilterList(FilterLists $list, FilterSources $source, int return $this->createJob('filter:update', ['list' => $list->value, 'source' => $source->value], $packageId, $executeAfter); } - private function getPendingUpdateJob(int $packageId, bool $updateEqualRefs = false, bool $deleteBefore = false): ?string + private function getPendingUpdateJob(int $packageId, bool $updateSourceDistUrl = false, bool $deleteBefore = false): ?string { $result = $this->getEM()->getConnection()->fetchAssociative( 'SELECT id, payload FROM job WHERE packageId = :package AND status = :status AND type = :type LIMIT 1', @@ -99,7 +99,7 @@ private function getPendingUpdateJob(int $packageId, bool $updateEqualRefs = fal if ($result) { $payload = json_decode($result['payload'], true); - if ($payload['update_equal_refs'] === $updateEqualRefs && $payload['delete_before'] === $deleteBefore) { + if (($payload['update_source_dist_url'] ?? false) === $updateSourceDistUrl && $payload['delete_before'] === $deleteBefore) { return $result['id']; } } diff --git a/src/Service/UpdaterWorker.php b/src/Service/UpdaterWorker.php index 5987e1c3e..5af446c87 100644 --- a/src/Service/UpdaterWorker.php +++ b/src/Service/UpdaterWorker.php @@ -196,8 +196,8 @@ public function process(Job $job, SignalHandler $signal): array try { $flags = 0; $useVersionCache = true; - if ($job->getPayload()['update_equal_refs'] === true) { - $flags = Updater::UPDATE_EQUAL_REFS; + if (($job->getPayload()['update_source_dist_url'] ?? false) === true) { + $flags = Updater::UPDATE_SOURCE_DIST_URL; $useVersionCache = false; } if ($job->getPayload()['delete_before'] === true) { diff --git a/templates/about/version_immutability.html.twig b/templates/about/version_immutability.html.twig new file mode 100644 index 000000000..c1297ccd2 --- /dev/null +++ b/templates/about/version_immutability.html.twig @@ -0,0 +1,37 @@ +{% extends "layout.html.twig" %} + +{% block content %} +

Version immutability

+ +

Once a stable (non-dev) version of a package is published on Packagist, its source and dist + reference are locked. The Packagist crawler will not update them, even if the upstream tag is moved or rewritten in the + git repository. Dev branches (versions matching dev-* or *-dev) continue to track their branches + as they always have — this rule applies only to stable releases.

+ +

Why?

+

A stable release is a contract: every downstream user who installed vendor/pkg:1.2.3 expects to receive the + exact same code as everyone else, today and a year from now. Allowing the source reference of a published version to + change opens the door to supply-chain attacks (e.g. taking over a tag after release) and to maintainer footguns + (e.g. a silent re-tag introducing a regression that nobody can audit). Making stable versions immutable closes both.

+ +

What you'll see if a re-tag is detected

+
    +
  • A warning is shown in the update log on the package page.
    • +
  • An email is sent to the package maintainers explaining the previous and attempted reference.
    • +
  • An audit record is written.
    • +
  • The version page displays a badge noting that the upstream re-tag was blocked and that Packagist's stored + snapshot may no longer match what is currently in git.
    • +
+

The audit/email/badge fires at most once per attempted reference — subsequent crawls that observe the same + diverged reference do not re-send.

+ +

How to publish the change you want to make

+

Tag a new version with the fix. For example, if 1.2.3 had a regression, publish 1.2.4 (or even 1.2.3.1). + The new version will be crawled and made available as normal.

+ +

Takedowns and recovery

+

Maintainers can soft-delete a version they own from the package page. Such versions are hidden from Composer + metadata but still listed on the page (grayed out), and can be recovered by the maintainer at any time. Administrator + takedowns (for malware, abuse, or legal reasons) cannot be recovered by maintainers — if you believe a + takedown was applied in error, please contact Packagist support.

+{% endblock %} diff --git a/templates/audit_log/display/version_recovered.html.twig b/templates/audit_log/display/version_recovered.html.twig new file mode 100644 index 000000000..01e13fa17 --- /dev/null +++ b/templates/audit_log/display/version_recovered.html.twig @@ -0,0 +1,5 @@ +{% import 'audit_log/macros.html.twig' as auditLog %} + +{{ auditLog.packageLink(display.packageName) }} {{ display.version }}
+Recovered from previous soft-delete ({{ display.previousReason }})
+By: {{ display.actor.username }} diff --git a/templates/audit_log/display/version_reference_change_blocked.html.twig b/templates/audit_log/display/version_reference_change_blocked.html.twig new file mode 100644 index 000000000..20e5a897d --- /dev/null +++ b/templates/audit_log/display/version_reference_change_blocked.html.twig @@ -0,0 +1,6 @@ +{% import 'audit_log/macros.html.twig' as auditLog %} + +{{ auditLog.packageLink(display.packageName) }} {{ display.version }}
+Re-tag blocked — upstream attempted to change the source/dist reference of a stable version.
+From: {{ display.refFrom ?: '' }}
+To: {{ display.refTo }} diff --git a/templates/audit_log/display/version_soft_deleted.html.twig b/templates/audit_log/display/version_soft_deleted.html.twig new file mode 100644 index 000000000..08d492f4d --- /dev/null +++ b/templates/audit_log/display/version_soft_deleted.html.twig @@ -0,0 +1,5 @@ +{% import 'audit_log/macros.html.twig' as auditLog %} + +{{ auditLog.packageLink(display.packageName) }} {{ display.version }}
+Soft-deleted ({{ display.reason }}){% if display.reasonText %}: {{ display.reasonText }}{% endif %}
+By: {{ display.actor.username }} diff --git a/templates/email/version_reference_change_blocked.txt.twig b/templates/email/version_reference_change_blocked.txt.twig new file mode 100644 index 000000000..b98283cee --- /dev/null +++ b/templates/email/version_reference_change_blocked.txt.twig @@ -0,0 +1,26 @@ +{% autoescape false -%} +The {{ package.name }} package of which you are a maintainer had an attempted +update to version {{ version }} blocked, because a published stable version's +source/dist reference changed in your git repository. + + Previously published reference: {{ oldRef ?: '' }} + Newly observed reference: {{ newRef }} + +Stable versions on Packagist are immutable: once published, their source and dist +references cannot change. This protects downstream users from a published version +silently swapping its contents (e.g. via a re-tag or force-push). + +If the change in your repository was intentional and you want the new code to be +available to users, please tag a new version with the fix and push it. + +Ideally you should also restore the tag to the previous commit reference to +bring the repository back in line with what Packagist.org shows and avoid +confusing people. + +Package page: {{ packageUrl }} +More information: {{ docUrl }} + +-- +If you do not wish to receive such emails in the future you can disable +notifications on your profile page: https://packagist.org/profile/edit +{%- endautoescape %} diff --git a/templates/package/version_list.html.twig b/templates/package/version_list.html.twig index cff73ea08..ce2e1b07e 100644 --- a/templates/package/version_list.html.twig +++ b/templates/package/version_list.html.twig @@ -2,7 +2,9 @@
    {% for version in versions %} {% set expanded = version.id == expandedId|default(false) %} -
  • + {% set softDeleted = version.softDeletedAt is not null %} + {% set reason = version.deletionReason %} +
  • {{- version.version -}} {% if version.hasVersionAlias() %} @@ -22,11 +24,59 @@ {% endif %} - {% if deleteVersionCsrfToken is defined and deleteVersionCsrfToken is not empty %} -
    - - -
    + {% if version.lastBlockedReference is not null and not softDeleted %} + + + + {% endif %} + + {% if softDeleted %} + {% set deletionTitle = 'Deleted' %} + {% set deletionIcon = 'glyphicon-trash' %} + {% if reason == enum('App\\Audit\\VersionDeletionReason').DeletedByMaintainer %} + {% set deletionTitle = 'Deleted by maintainer on ' ~ version.softDeletedAt|date('Y-m-d H:i:s') ~ ' UTC' %} + {% elseif reason == enum('App\\Audit\\VersionDeletionReason').DeletedByAdmin %} + {% set deletionTitle = 'Removed by admin on ' ~ version.softDeletedAt|date('Y-m-d H:i:s') ~ ' UTC' ~ (version.deletionReasonText ? ': ' ~ version.deletionReasonText : '') %} + {% elseif reason == enum('App\\Audit\\VersionDeletionReason').AutoDeletedMissing %} + {% set deletionTitle = 'No longer found in upstream repository' %} + {% elseif reason == enum('App\\Audit\\VersionDeletionReason').Hidden %} + {% set deletionTitle = 'Hidden by admin on ' ~ version.softDeletedAt|date('Y-m-d H:i:s') ~ ' UTC' ~ (version.deletionReasonText ? ': ' ~ version.deletionReasonText : '') %} + {% set deletionIcon = 'glyphicon-eye-close' %} + {% endif %} + + + + {% endif %} + + {% if not softDeleted and canDeleteVersion|default(false) %} + {% if canHideVersion|default(false) %} +
    + + +
    + {% endif %} +
    + + +
    + {% endif %} + + {% set canRecover = false %} + {% if softDeleted and canDeleteVersion|default(false) %} + {% if reason is null or reason.isRecoverableByMaintainer() or is_granted(enum('App\\Security\\Voter\\PackageActions').AdminDeleteVersion.value, package) %} + {% set canRecover = true %} + {% endif %} + {% endif %} + {% if canRecover %} +
    + + +
    {% endif %}
    • {% endfor %} diff --git a/templates/package/view_package.html.twig b/templates/package/view_package.html.twig index 7079ce01b..55be0e119 100644 --- a/templates/package/view_package.html.twig +++ b/templates/package/view_package.html.twig @@ -377,7 +377,10 @@ {% include 'package/version_list.html.twig' with { versions: versions, expandedId: expandedVersion.id, - deleteVersionCsrfToken: deleteVersionCsrfToken|default(null), + manageVersionsCsrfToken: manageVersionsCsrfToken|default(null), + canDeleteVersion: canDeleteVersion|default(false), + canAdminDeleteVersion: canAdminDeleteVersion|default(false), + canHideVersion: canHideVersion|default(false), package: package, showUpdated: true, showUpdateButton: not hasActions and app.user and not package.wasUpdatedInTheLast24Hours(), diff --git a/tests/Entity/VersionRepositoryTest.php b/tests/Entity/VersionRepositoryTest.php index e1e672e9b..e3f56aea3 100644 --- a/tests/Entity/VersionRepositoryTest.php +++ b/tests/Entity/VersionRepositoryTest.php @@ -13,6 +13,7 @@ namespace App\Tests\Entity; use App\Audit\AuditRecordType; +use App\Audit\VersionDeletionReason; use App\Entity\AuditRecord; use App\Entity\Version; use App\Entity\VersionRepository; @@ -70,4 +71,110 @@ public function testRemoveVersionMarksForRemovalWithAuditRecord(bool $createAudi $this->assertNull($auditRecord, 'Audit record for version deleted created'); } } + + public function testSoftDeleteMarksReasonAndWritesAudit(): void + { + $em = self::getEM(); + $version = $this->seedStableVersion('vendor/sd', '2.0.0', '2.0.0.0'); + + $this->versionRepository->softDelete($version, VersionDeletionReason::DeletedByMaintainer, null, null); + $em->flush(); + $em->clear(); + + $reloaded = $this->versionRepository->find($version->getId()); + self::assertNotNull($reloaded); + self::assertNotNull($reloaded->getSoftDeletedAt()); + self::assertSame(VersionDeletionReason::DeletedByMaintainer, $reloaded->getDeletionReason()); + self::assertNull($reloaded->getDeletionReasonText()); + + $audit = $em->getRepository(AuditRecord::class)->findOneBy([ + 'type' => AuditRecordType::VersionSoftDeleted->value, + 'packageId' => $reloaded->getPackage()->getId(), + ]); + self::assertNotNull($audit, 'softDelete() should write a VersionSoftDeleted audit row'); + self::assertSame(VersionDeletionReason::DeletedByMaintainer->value, $audit->attributes['reason']); + } + + public function testSoftDeletePersistsAdminReasonText(): void + { + $em = self::getEM(); + $version = $this->seedStableVersion('vendor/sd-admin', '2.0.0', '2.0.0.0'); + + $this->versionRepository->softDelete($version, VersionDeletionReason::DeletedByAdmin, 'legal takedown', null); + $em->flush(); + $em->clear(); + + $reloaded = $this->versionRepository->find($version->getId()); + self::assertNotNull($reloaded); + self::assertSame(VersionDeletionReason::DeletedByAdmin, $reloaded->getDeletionReason()); + self::assertSame('legal takedown', $reloaded->getDeletionReasonText()); + + $audit = $em->getRepository(AuditRecord::class)->findOneBy([ + 'type' => AuditRecordType::VersionSoftDeleted->value, + 'packageId' => $reloaded->getPackage()->getId(), + ]); + self::assertNotNull($audit); + self::assertSame('legal takedown', $audit->attributes['reasonText']); + } + + public function testRecoverClearsAllSoftDeleteState(): void + { + $em = self::getEM(); + $version = $this->seedStableVersion('vendor/recover', '2.0.0', '2.0.0.0'); + + $this->versionRepository->softDelete($version, VersionDeletionReason::DeletedByMaintainer, null, null); + $em->flush(); + + $this->versionRepository->recover($version, null); + $em->flush(); + $em->clear(); + + $reloaded = $this->versionRepository->find($version->getId()); + self::assertNotNull($reloaded); + self::assertNull($reloaded->getSoftDeletedAt()); + self::assertNull($reloaded->getDeletionReason()); + self::assertNull($reloaded->getDeletionReasonText()); + + $audit = $em->getRepository(AuditRecord::class)->findOneBy([ + 'type' => AuditRecordType::VersionRecovered->value, + 'packageId' => $reloaded->getPackage()->getId(), + ]); + self::assertNotNull($audit, 'recover() should write a VersionRecovered audit row'); + self::assertSame(VersionDeletionReason::DeletedByMaintainer->value, $audit->attributes['previousReason']); + } + + public function testGetVersionMetadataForUpdateIncludesNewProjection(): void + { + $em = self::getEM(); + $version = $this->seedStableVersion('vendor/projection', '2.0.0', '2.0.0.0'); + $version->setLastBlockedReference('aabbccdd'); + $em->persist($version); + $em->flush(); + + $meta = $this->versionRepository->getVersionMetadataForUpdate($version->getPackage()); + self::assertArrayHasKey('2.0.0.0', $meta); + self::assertArrayHasKey('dist', $meta['2.0.0.0']); + self::assertArrayHasKey('deletionReason', $meta['2.0.0.0']); + self::assertArrayHasKey('lastBlockedReference', $meta['2.0.0.0']); + self::assertSame('aabbccdd', $meta['2.0.0.0']['lastBlockedReference']); + } + + private function seedStableVersion(string $packageName, string $version, string $normalized): Version + { + $package = self::createPackage($packageName, 'https://github.com/'.$packageName); + + $v = new Version(); + $v->setPackage($package); + $v->setName($package->getName()); + $v->setVersion($version); + $v->setNormalizedVersion($normalized); + $v->setDevelopment(false); + $v->setLicense([]); + $v->setAutoload([]); + $package->getVersions()->add($v); + + $this->store($package, $v); + + return $v; + } } diff --git a/tests/Package/UpdaterTest.php b/tests/Package/UpdaterTest.php index 4c056b250..7b4acb356 100644 --- a/tests/Package/UpdaterTest.php +++ b/tests/Package/UpdaterTest.php @@ -12,8 +12,14 @@ namespace App\Tests\Package; +use App\Audit\AuditRecordType; +use App\Audit\VersionDeletionReason; +use App\Entity\AuditRecord; use App\Entity\Package; use App\Entity\PackageReadme; +use App\Entity\Version; +use App\Entity\VersionRepository; +use App\Model\PackageManager; use App\Model\ProviderManager; use App\Model\VersionIdCache; use App\Package\Updater; @@ -27,9 +33,9 @@ use Composer\Repository\Vcs\VcsDriverInterface; use Composer\Repository\VcsRepository; use Doctrine\Persistence\ManagerRegistry; -use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\Stub; +use Psr\Log\NullLogger; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Mailer\MailerInterface; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; @@ -58,21 +64,26 @@ protected function setUp(): void $registry = static::getContainer()->get(ManagerRegistry::class); $providerManagerMock = $this->createStub(ProviderManager::class); - $this->driverMock = $this->createMock(GitDriver::class); $package = new CompletePackage('test/pkg', '1.0.0.0', '1.0.0'); $this->repositoryMock->method('getPackages')->willReturn([ $package, ]); - $this->repositoryMock->method('getDriver')->willReturn($this->driverMock); $versionIdCache = $this->createStub(VersionIdCache::class); $mailerMock = $this->createStub(MailerInterface::class); $routerMock = $this->createStub(UrlGeneratorInterface::class); $eventDispatcherMock = $this->createStub(EventDispatcher::class); + $packageManagerMock = $this->createStub(PackageManager::class); + + $this->updater = new Updater($registry, $providerManagerMock, $versionIdCache, $mailerMock, 'foo@example.org', $routerMock, $eventDispatcherMock, $packageManagerMock, new NullLogger()); + } - $this->updater = new Updater($registry, $providerManagerMock, $versionIdCache, $mailerMock, 'foo@example.org', $routerMock, $eventDispatcherMock); + private function bindReadmeDriver(): void + { + $this->driverMock = $this->createMock(GitDriver::class); + $this->repositoryMock->method('getDriver')->willReturn($this->driverMock); } protected function tearDown(): void @@ -83,6 +94,7 @@ protected function tearDown(): void public function testUpdatesTheReadme(): void { + $this->bindReadmeDriver(); $this->driverMock->method('getRootIdentifier')->willReturn('master'); $this->driverMock->method('getComposerInformation') ->willReturn(['readme' => 'README.md']); @@ -117,6 +129,7 @@ public function testConvertsMarkdownForReadme(): void EOR; + $this->bindReadmeDriver(); $this->driverMock->method('getRootIdentifier')->willReturn('master'); $this->driverMock->method('getComposerInformation') ->willReturn(['readme' => 'README.md']); @@ -147,6 +160,7 @@ public function testNoUsefulTitlesAreRemovedForReadme(): void EOR; + $this->bindReadmeDriver(); $this->driverMock->method('getRootIdentifier')->willReturn('master'); $this->driverMock->method('getComposerInformation') ->willReturn(['readme' => 'README.md']); @@ -161,6 +175,7 @@ public function testNoUsefulTitlesAreRemovedForReadme(): void public function testSurroundsTextReadme(): void { + $this->bindReadmeDriver(); $this->driverMock->method('getRootIdentifier')->willReturn('master'); $this->driverMock->method('getComposerInformation') ->willReturn(['readme' => 'README.txt']); @@ -175,6 +190,7 @@ public function testSurroundsTextReadme(): void public function testUnderstandsDifferentFileNames(): void { + $this->bindReadmeDriver(); $this->driverMock->method('getRootIdentifier')->willReturn('master'); $this->driverMock->method('getComposerInformation') ->willReturn(['readme' => 'liesmich']); @@ -187,7 +203,6 @@ public function testUnderstandsDifferentFileNames(): void self::assertSame('
    This is the readme
    ', $readme->contents); } - #[AllowMockObjectsWithoutExpectations] public function testReadmeParsing(): void { $readme = <<<'SOURCE' @@ -313,4 +328,259 @@ public function testReadmeParsing(): void EXPECTED, $readme); } + + public function testStableVersionWithUnchangedRefStaysSkipped(): void + { + $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $packageManagerMock = $this->createMock(PackageManager::class); + $packageManagerMock->expects($this->never())->method('notifyVersionReferenceChangeBlocked'); + $this->rebuildUpdater($packageManagerMock); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $blocked = $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked); + self::assertSame(0, $blocked); + } + + public function testStableVersionWithChangedRefIsBlockedAndAudited(): void + { + $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', '9999999999999999'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $packageManagerMock = $this->createMock(PackageManager::class); + $packageManagerMock->expects($this->once())->method('notifyVersionReferenceChangeBlocked') + ->with($this->package, '1.0.0', 'abcdef1234567890', '9999999999999999'); + $this->rebuildUpdater($packageManagerMock); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em = self::getEM(); + $em->clear(); + $reloaded = $em->getRepository(Version::class)->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => '1.0.0.0']); + self::assertNotNull($reloaded); + self::assertSame('abcdef1234567890', $reloaded->getSource()['reference'] ?? null, 'stored source ref must not change on block'); + self::assertSame('9999999999999999', $reloaded->getLastBlockedReference()); + + $audit = $em->getRepository(AuditRecord::class)->findOneBy([ + 'type' => AuditRecordType::VersionReferenceChangeBlocked->value, + 'packageId' => $this->package->getId(), + ]); + self::assertNotNull($audit); + self::assertSame('abcdef1234567890', $audit->attributes['ref_from']); + self::assertSame('9999999999999999', $audit->attributes['ref_to']); + } + + public function testStableBlockedTwiceWithSameRefDoesNotDuplicateAudit(): void + { + $version = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $version->setLastBlockedReference('9999999999999999'); + self::getEM()->persist($version); + self::getEM()->flush(); + + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', '9999999999999999'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $packageManagerMock = $this->createMock(PackageManager::class); + $packageManagerMock->expects($this->never())->method('notifyVersionReferenceChangeBlocked'); + $this->rebuildUpdater($packageManagerMock); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked)); + } + + public function testStableRevertClearsLastBlockedReference(): void + { + $version = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $version->setLastBlockedReference('9999999999999999'); + self::getEM()->persist($version); + self::getEM()->flush(); + + // upstream goes back to the original ref + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $packageManagerMock = $this->createMock(PackageManager::class); + $packageManagerMock->expects($this->never())->method('notifyVersionReferenceChangeBlocked'); + $this->rebuildUpdater($packageManagerMock); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em = self::getEM(); + $em->clear(); + $reloaded = $em->getRepository(Version::class)->findOneBy(['name' => 'test/pkg']); + self::assertNotNull($reloaded); + self::assertNull($reloaded->getLastBlockedReference(), 'lastBlockedReference must be cleared when upstream reverts'); + self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked)); + } + + public function testIntentionallySoftDeletedStableIsNotRecreated(): void + { + $version = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $version->setSoftDeletedAt(new \DateTimeImmutable('-2 hours')); + $version->setDeletionReason(VersionDeletionReason::DeletedByMaintainer); + self::getEM()->persist($version); + self::getEM()->flush(); + + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em = self::getEM(); + $em->clear(); + $reloaded = $em->getRepository(Version::class)->findOneBy(['name' => 'test/pkg']); + self::assertNotNull($reloaded); + self::assertNotNull($reloaded->getSoftDeletedAt(), 'maintainer-soft-deleted row must stay soft-deleted across an update run'); + self::assertSame(VersionDeletionReason::DeletedByMaintainer, $reloaded->getDeletionReason()); + } + + public function testBrandNewVersionWithoutEffectiveRefIsRejected(): void + { + // construct an upstream version with neither source nor dist reference + $upstream = new CompletePackage('test/pkg', '1.0.0.0', '1.0.0'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em = self::getEM(); + $em->clear(); + $row = $em->getRepository(Version::class)->findOneBy(['name' => 'test/pkg']); + self::assertNull($row, 'a version with no usable reference must not be created'); + } + + public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithNewRef(): void + { + $existing = $this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'oldref1234567890'); + $existing->setSoftDeletedAt(new \DateTimeImmutable('-2 hours')); + $existing->setDeletionReason(VersionDeletionReason::AutoDeletedMissing); + self::getEM()->persist($existing); + self::getEM()->flush(); + + $upstream = $this->buildCompletePackage('test/pkg', 'dev-main', 'dev-main', 'newref9999999999'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em = self::getEM(); + $em->clear(); + $reloaded = $em->getRepository(Version::class)->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => 'dev-main']); + self::assertNotNull($reloaded); + self::assertNull($reloaded->getSoftDeletedAt(), 'dev version must be recovered when branch reappears'); + self::assertNull($reloaded->getDeletionReason()); + self::assertSame('newref9999999999', $reloaded->getSource()['reference'] ?? null); + } + + public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithUnchangedRef(): void + { + $existing = $this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'sameref1234567890'); + $existing->setSoftDeletedAt(new \DateTimeImmutable('-2 hours')); + $existing->setDeletionReason(VersionDeletionReason::AutoDeletedMissing); + self::getEM()->persist($existing); + self::getEM()->flush(); + + // upstream re-appears with the *same* ref the row had before being auto-soft-deleted + $upstream = $this->buildCompletePackage('test/pkg', 'dev-main', 'dev-main', 'sameref1234567890'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em = self::getEM(); + $em->clear(); + $reloaded = $em->getRepository(Version::class)->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => 'dev-main']); + self::assertNotNull($reloaded); + self::assertNull($reloaded->getSoftDeletedAt(), 'auto-soft-deleted dev rows must be recovered even when the ref is unchanged'); + self::assertNull($reloaded->getDeletionReason()); + } + + private function stableDriver(): VcsDriverInterface&Stub + { + $driver = $this->createStub(GitDriver::class); + $driver->method('getRootIdentifier')->willReturn('master'); + $driver->method('getComposerInformation')->willReturn([]); + + return $driver; + } + + private function seedStableVersion(Package $package, string $prettyVersion, string $normalizedVersion, ?string $sourceRef): Version + { + return $this->seedVersion($package, $prettyVersion, $normalizedVersion, $sourceRef, false); + } + + private function seedDevVersion(Package $package, string $prettyVersion, string $normalizedVersion, ?string $sourceRef): Version + { + return $this->seedVersion($package, $prettyVersion, $normalizedVersion, $sourceRef, true); + } + + private function seedVersion(Package $package, string $prettyVersion, string $normalizedVersion, ?string $sourceRef, bool $isDev): Version + { + $em = self::getEM(); + $v = new Version(); + $v->setPackage($package); + $v->setName($package->getName()); + $v->setVersion($prettyVersion); + $v->setNormalizedVersion($normalizedVersion); + $v->setDevelopment($isDev); + $v->setLicense([]); + $v->setAutoload([]); + if ($sourceRef !== null) { + $v->setSource(['type' => 'git', 'url' => 'https://example.com/test/pkg', 'reference' => $sourceRef]); + } + $em->persist($v); + $em->flush(); + + return $v; + } + + private function buildCompletePackage(string $name, string $prettyVersion, string $normalizedVersion, string $sourceRef): CompletePackage + { + $p = new CompletePackage($name, $normalizedVersion, $prettyVersion); + $p->setSourceType('git'); + $p->setSourceUrl('https://example.com/'.$name); + $p->setSourceReference($sourceRef); + + return $p; + } + + private function rebuildUpdater(PackageManager $packageManager): void + { + $registry = static::getContainer()->get(ManagerRegistry::class); + $providerManagerMock = $this->createStub(ProviderManager::class); + $versionIdCache = $this->createStub(VersionIdCache::class); + $mailerMock = $this->createStub(MailerInterface::class); + $routerMock = $this->createStub(UrlGeneratorInterface::class); + $eventDispatcherMock = $this->createStub(EventDispatcher::class); + + $this->updater = new Updater($registry, $providerManagerMock, $versionIdCache, $mailerMock, 'foo@example.org', $routerMock, $eventDispatcherMock, $packageManager, new NullLogger()); + } + + private function countAudits(AuditRecordType $type): int + { + return \count(self::getEM()->getRepository(AuditRecord::class)->findBy([ + 'type' => $type->value, + 'packageId' => $this->package->getId(), + ])); + } } diff --git a/tests/Package/V2DumperTest.php b/tests/Package/V2DumperTest.php index 2948612aa..a3f0a9de1 100644 --- a/tests/Package/V2DumperTest.php +++ b/tests/Package/V2DumperTest.php @@ -199,6 +199,28 @@ public function testSpamFrozenPackageIsSkipped(): void $this->assertFileDoesNotExist($releaseFile); } + public function testSoftDeletedVersionIsExcludedFromMetadata(): void + { + $package = self::createPackage('acme/pkg-sd', 'https://example.com/acme/pkg-sd'); + $kept = $this->createVersion($package, '1.0.0'); + $deleted = $this->createVersion($package, '1.1.0'); + $deleted->setSoftDeletedAt(new \DateTimeImmutable()); + $deleted->setDeletionReason(\App\Audit\VersionDeletionReason::DeletedByMaintainer); + $this->store($package, $kept, $deleted); + + $this->dumper->dump([$package->getId()], force: true); + + $releaseFile = $this->buildDir.'/p2/acme/pkg-sd.json'; + $this->assertFileExists($releaseFile); + + $data = json_decode((string) file_get_contents($releaseFile), true); + $entries = $data['packages']['acme/pkg-sd']; + $versionsInDump = array_map(static fn (array $row): string => $row['version'] ?? '', $entries); + + $this->assertContains('1.0.0', $versionsInDump); + $this->assertNotContains('1.1.0', $versionsInDump, 'soft-deleted version must not appear in metadata dump'); + } + public function testDumpRootAdvertisesFilterSummaryUrl(): void { $this->dumper->dumpRoot(); diff --git a/translations/messages.en.yml b/translations/messages.en.yml index 3e4136b5e..ad4302cf9 100644 --- a/translations/messages.en.yml +++ b/translations/messages.en.yml @@ -198,7 +198,10 @@ audit_log: username_changed: Username changed version_created: Version created version_deleted: Version deleted + version_soft_deleted: Version soft-deleted + version_recovered: Version recovered version_reference_changed: Version reference changed + version_reference_change_blocked: Version reference change blocked filter_list_entry_added: Entry added filter_list_entry_deleted: Entry deleted enums: From 348d5fa98146182c96fb43fe69d1157f96a8ee77 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 27 May 2026 13:21:51 +0200 Subject: [PATCH 02/13] Also remove soft-deleted versions from package API endpoint --- src/Entity/Package.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Entity/Package.php b/src/Entity/Package.php index 10f62831c..ebdc6fe89 100644 --- a/src/Entity/Package.php +++ b/src/Entity/Package.php @@ -255,7 +255,12 @@ public function toArray(VersionRepository|array $versionRepo, bool $serializeFor $versions = $versionRepo; } else { $versions = []; - $partialVersions = $this->getVersions()->toArray(); + // Soft-deleted versions are kept in the DB for the UI to show grayed out but must not + // leak via the public JSON endpoint — the V2 dumper applies the same filter. + $partialVersions = array_values(array_filter( + $this->getVersions()->toArray(), + static fn (Version $v): bool => !$v->isSoftDeleted() + )); while ($partialVersions) { $versionRepo->getEntityManager()->clear(); From 27f4f2877027ddeb74563bdf251134694f98c947 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 27 May 2026 13:32:06 +0200 Subject: [PATCH 03/13] Make sure delete_before sees all versions --- src/Package/Updater.php | 15 +++++++++--- tests/Package/UpdaterTest.php | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/Package/Updater.php b/src/Package/Updater.php index f74b0fe08..dac0a6a26 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -281,10 +281,17 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep // hard-deleted. DELETE_BEFORE wipes the dev-branch trackers and lets the normal update // path reconcile stable rows via the immutability gate (matching refs are skipped, // diverging refs are blocked + audited). - foreach ($package->getVersions() as $version) { - if ($version->isDevelopment()) { - $versionRepository->remove($version); - } + // Source the rows from the DB rather than $package->getVersions() — the in-memory + // collection state depends on how the caller loaded the package, and we must not + // silently skip removals just because the collection isn't lazy-loaded yet. + /** @var list $devVersions */ + $devVersions = $versionRepository->createQueryBuilder('v') + ->where('v.package = :package') + ->andWhere('v.development = true') + ->setParameter('package', $package) + ->getQuery()->getResult(); + foreach ($devVersions as $version) { + $versionRepository->remove($version); } $em->flush(); diff --git a/tests/Package/UpdaterTest.php b/tests/Package/UpdaterTest.php index 7b4acb356..def629ef6 100644 --- a/tests/Package/UpdaterTest.php +++ b/tests/Package/UpdaterTest.php @@ -491,6 +491,52 @@ public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithN self::assertSame('newref9999999999', $reloaded->getSource()['reference'] ?? null); } + public function testDeleteBeforeWipesDevRowsButPreservesStableAndSoftDeletedStable(): void + { + // Seed three rows: an active stable, a maintainer-soft-deleted stable, and a dev branch. + // DELETE_BEFORE must wipe only the dev row, leave both stable rows untouched, and the + // post-loop prune must not crash on the survivors after $em->refresh($package). + $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $softDeleted = $this->seedStableVersion($this->package, '1.1.0', '1.1.0.0', '1234567890abcdef'); + $softDeleted->setSoftDeletedAt(new \DateTimeImmutable('-2 hours')); + $softDeleted->setDeletionReason(VersionDeletionReason::DeletedByMaintainer); + $this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'devref1234567890'); + self::getEM()->persist($softDeleted); + self::getEM()->flush(); + + // Upstream only returns the active stable version — dev-main has disappeared from upstream + // and 1.1.0 is also missing (consistent with the maintainer pull). + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $packageManagerMock = $this->createMock(PackageManager::class); + $packageManagerMock->expects($this->never())->method('notifyVersionReferenceChangeBlocked'); + $this->rebuildUpdater($packageManagerMock); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock, Updater::DELETE_BEFORE); + + $em = self::getEM(); + $em->clear(); + $versionRepo = $em->getRepository(Version::class); + + $active = $versionRepo->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => '1.0.0.0']); + self::assertNotNull($active, 'active stable row must survive DELETE_BEFORE'); + self::assertNull($active->getSoftDeletedAt()); + self::assertSame('abcdef1234567890', $active->getSource()['reference'] ?? null); + + $reloadedSoftDeleted = $versionRepo->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => '1.1.0.0']); + self::assertNotNull($reloadedSoftDeleted, 'maintainer-soft-deleted stable row must survive DELETE_BEFORE'); + self::assertNotNull($reloadedSoftDeleted->getSoftDeletedAt(), 'soft-delete marker must stay'); + self::assertSame(VersionDeletionReason::DeletedByMaintainer, $reloadedSoftDeleted->getDeletionReason()); + + $dev = $versionRepo->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => 'dev-main']); + self::assertNull($dev, 'dev row must be hard-deleted by DELETE_BEFORE'); + + self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked)); + } + public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithUnchangedRef(): void { $existing = $this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'sameref1234567890'); From 35a6bd7ed09e34681bcde2a52b49294e4eac549e Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 27 May 2026 13:45:29 +0200 Subject: [PATCH 04/13] Go through UoW vs direct SQL for url updates --- src/Package/Updater.php | 46 ++++++++++++------ tests/Package/UpdaterTest.php | 92 +++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 14 deletions(-) diff --git a/src/Package/Updater.php b/src/Package/Updater.php index dac0a6a26..d6d4459fa 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -62,6 +62,7 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Contracts\EventDispatcher\Event; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Webmozart\Assert\Assert; final readonly class VersionUpdatedResult { @@ -458,7 +459,7 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep * * @param array{id: int, version: string, normalizedVersion: string, development: int, source: array{type: string|null, url: string|null, reference: string|null}|null, dist: array{type: string|null, url: string|null, reference: string|null, shasum: string|null}|null, softDeletedAt: string|null, deletionReason: string|null, lastBlockedReference: string|null, defaultBranch: int} $existingVersion */ - private function applyStableImmutabilityGate(IOInterface $io, \Doctrine\ORM\EntityManagerInterface $em, Package $package, array $existingVersion, CompletePackageInterface $data, int $flags, VcsDriverInterface $driver, ?string $newEffectiveRef): VersionSkippedResult + private function applyStableImmutabilityGate(IOInterface $io, \Doctrine\ORM\EntityManagerInterface $em, VersionRepository $versionRepo, Package $package, array $existingVersion, CompletePackageInterface $data, int $flags, VcsDriverInterface $driver, ?string $newEffectiveRef): VersionSkippedResult { $normVersion = $data->getVersion(); $prettyVersion = $data->getPrettyVersion(); @@ -484,7 +485,7 @@ private function applyStableImmutabilityGate(IOInterface $io, \Doctrine\ORM\Enti } if ($flags & self::UPDATE_SOURCE_DIST_URL) { - $this->applySourceDistUrlRewrite($io, $em, $existingVersion, $data, $driver); + $this->applySourceDistUrlRewrite($io, $versionRepo, $existingVersion, $data, $driver); } return new VersionSkippedResult(id: $existingVersion['id'], version: strtolower($normVersion)); @@ -516,7 +517,7 @@ private function applyStableImmutabilityGate(IOInterface $io, \Doctrine\ORM\Enti * * @param array{id: int, source: array{type: string|null, url: string|null, reference: string|null}|null, dist: array{type: string|null, url: string|null, reference: string|null, shasum: string|null}|null} $existingVersion */ - private function applySourceDistUrlRewrite(IOInterface $io, \Doctrine\ORM\EntityManagerInterface $em, array $existingVersion, CompletePackageInterface $data, VcsDriverInterface $driver): void + private function applySourceDistUrlRewrite(IOInterface $io, VersionRepository $versionRepo, array $existingVersion, CompletePackageInterface $data, VcsDriverInterface $driver): void { $prettyVersion = $data->getPrettyVersion(); $skip = function (string $reason) use ($io, $prettyVersion, $data, $existingVersion): void { @@ -573,19 +574,36 @@ private function applySourceDistUrlRewrite(IOInterface $io, \Doctrine\ORM\Entity return; } - $newSource = ($oldSource ?? []) + []; + $version = $versionRepo->find($existingVersion['id']); + if (null === $version) { + $skip('version not found by id'); + + return; + } + + Assert::notNull($oldSource); + Assert::notNull($oldDist); + $oldSourceUrl = $oldSource['url'] ?? null; + $oldDistUrlStored = $oldDist['url'] ?? null; + + $newSource = $oldSource; $newSource['url'] = $newSourceUrl; - $newDist = ($oldDist ?? []) + []; + $newDist = $oldDist; $newDist['url'] = $newDistUrl; - $em->getConnection()->executeStatement( - 'UPDATE package_version SET source = :source, dist = :dist WHERE id = :id', - [ - 'source' => json_encode($newSource, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE), - 'dist' => json_encode($newDist, \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE), - 'id' => $existingVersion['id'], - ] - ); + $version->setSource($newSource); + $version->setDist($newDist); + + $this->logger->info('Rewrote source/dist URL for stable version', [ + 'package' => $data->getName(), + 'version' => $prettyVersion, + 'version_id' => $existingVersion['id'], + 'source_url_from' => $oldSourceUrl, + 'source_url_to' => $newSourceUrl, + 'dist_url_from' => $oldDistUrlStored, + 'dist_url_to' => $newDistUrl, + 'reference' => $newSourceRef, + ]); } /** @@ -625,7 +643,7 @@ private function updateInformation(IOInterface $io, VersionRepository $versionRe // Stable-version immutability gate: any existing stable version is frozen. if (!$data->isDev()) { - return $this->applyStableImmutabilityGate($io, $em, $package, $existingVersion, $data, $flags, $driver, $newEffectiveRef); + return $this->applyStableImmutabilityGate($io, $em, $versionRepo, $package, $existingVersion, $data, $flags, $driver, $newEffectiveRef); } } elseif ($newEffectiveRef === null) { // Brand-new version with no usable identity: refuse to create. diff --git a/tests/Package/UpdaterTest.php b/tests/Package/UpdaterTest.php index def629ef6..e584c0874 100644 --- a/tests/Package/UpdaterTest.php +++ b/tests/Package/UpdaterTest.php @@ -537,6 +537,98 @@ public function testDeleteBeforeWipesDevRowsButPreservesStableAndSoftDeletedStab self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked)); } + public function testUpdateSourceDistUrlRewritesUrlsViaEntityAndKeepsRefsAndShasumIntact(): void + { + $em = self::getEM(); + $ref = str_repeat('a', 40); + $shasum = str_repeat('b', 64); + + $version = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', $ref); + $version->setSource(['type' => 'git', 'url' => 'https://old.example.com/test/pkg.git', 'reference' => $ref]); + $version->setDist(['type' => 'zip', 'url' => 'https://old.example.com/dist/'.$ref, 'reference' => $ref, 'shasum' => $shasum]); + $em->persist($version); + $em->flush(); + $versionId = $version->getId(); + + $upstream = new CompletePackage('test/pkg', '1.0.0.0', '1.0.0'); + $upstream->setSourceType('git'); + $upstream->setSourceUrl('https://new.example.com/test/pkg.git'); + $upstream->setSourceReference($ref); + $upstream->setDistType('zip'); + $upstream->setDistUrl('https://new.example.com/dist/'.$ref); + $upstream->setDistReference($ref); + // upstream shasum intentionally differs to prove the rewrite does NOT touch shasum + $upstream->setDistSha1Checksum(str_repeat('c', 64)); + + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + + $driver = $this->createStub(GitDriver::class); + $driver->method('getRootIdentifier')->willReturn('master'); + $driver->method('getComposerInformation')->willReturn([]); + $driver->method('getDist')->willReturn(['type' => 'zip', 'url' => 'https://new.example.com/dist/'.$ref, 'reference' => $ref, 'shasum' => $shasum]); + $this->repositoryMock->method('getDriver')->willReturn($driver); + + $packageManagerMock = $this->createMock(PackageManager::class); + $packageManagerMock->expects($this->never())->method('notifyVersionReferenceChangeBlocked'); + $this->rebuildUpdater($packageManagerMock); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock, Updater::UPDATE_SOURCE_DIST_URL); + + $em->clear(); + $reloaded = $em->getRepository(Version::class)->find($versionId); + self::assertNotNull($reloaded); + self::assertSame('https://new.example.com/test/pkg.git', $reloaded->getSource()['url'] ?? null); + self::assertSame($ref, $reloaded->getSource()['reference'] ?? null, 'source.reference must not change'); + self::assertSame('git', $reloaded->getSource()['type'] ?? null); + self::assertSame('https://new.example.com/dist/'.$ref, $reloaded->getDist()['url'] ?? null); + self::assertSame($ref, $reloaded->getDist()['reference'] ?? null, 'dist.reference must not change'); + self::assertSame($shasum, $reloaded->getDist()['shasum'] ?? null, 'dist.shasum must not be overwritten'); + + self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChanged)); + self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked)); + } + + public function testUpdateSourceDistUrlSkipsWhenDriverDistUrlDoesNotMatch(): void + { + $em = self::getEM(); + $ref = str_repeat('a', 40); + $shasum = str_repeat('b', 64); + + $version = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', $ref); + $version->setSource(['type' => 'git', 'url' => 'https://old.example.com/test/pkg.git', 'reference' => $ref]); + $version->setDist(['type' => 'zip', 'url' => 'https://old.example.com/dist/'.$ref, 'reference' => $ref, 'shasum' => $shasum]); + $em->persist($version); + $em->flush(); + $versionId = $version->getId(); + + $upstream = new CompletePackage('test/pkg', '1.0.0.0', '1.0.0'); + $upstream->setSourceType('git'); + $upstream->setSourceUrl('https://attacker.example.com/test/pkg.git'); + $upstream->setSourceReference($ref); + $upstream->setDistType('zip'); + $upstream->setDistUrl('https://attacker.example.com/dist/'.$ref); + $upstream->setDistReference($ref); + + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + + $driver = $this->createStub(GitDriver::class); + $driver->method('getRootIdentifier')->willReturn('master'); + $driver->method('getComposerInformation')->willReturn([]); + // driver-confirmed dist URL points elsewhere — the incoming claim must be rejected + $driver->method('getDist')->willReturn(['type' => 'zip', 'url' => 'https://different.example.com/dist/'.$ref, 'reference' => $ref, 'shasum' => $shasum]); + $this->repositoryMock->method('getDriver')->willReturn($driver); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock, Updater::UPDATE_SOURCE_DIST_URL); + + $em->clear(); + $reloaded = $em->getRepository(Version::class)->find($versionId); + self::assertNotNull($reloaded); + self::assertSame('https://old.example.com/test/pkg.git', $reloaded->getSource()['url'] ?? null, 'URL must not be rewritten when driver disagrees'); + self::assertSame('https://old.example.com/dist/'.$ref, $reloaded->getDist()['url'] ?? null); + } + public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithUnchangedRef(): void { $existing = $this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'sameref1234567890'); From 767629f7a6e0d90a8959c529730d5ca3673d2edc Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Wed, 27 May 2026 16:24:18 +0200 Subject: [PATCH 05/13] Fix picking of version for dependent/suggesters crawling --- src/Package/Updater.php | 14 +++++++++++--- tests/Package/UpdaterTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/Package/Updater.php b/src/Package/Updater.php index d6d4459fa..25cae2209 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -306,7 +306,7 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep $processedVersions = []; $idsToMarkUpdated = []; - /** @var int|false|null $dependentSuggesterSource Version id to use as dependent/suggester source */ + /** @var int|null $dependentSuggesterSource Version id to use as dependent/suggester source */ $dependentSuggesterSource = null; foreach ($versions as $version) { if ($version instanceof AliasPackage) { @@ -328,7 +328,7 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep continue; } - $versionId = false; + $versionId = null; $versionSoftDeleted = false; if ($result instanceof VersionUpdatedResult) { foreach ($result->events as $event) { @@ -345,6 +345,14 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep } else { $idsToMarkUpdated[] = $result->id; $versionSoftDeleted = $result->softDeleted; + // Skipped non-soft-deleted rows (immutable stable, ref-unchanged, ...) still need to + // be a valid dependent/suggester source — they ARE the canonical existing row for + // this version. Carrying $result->id here avoids pinning $dependentSuggesterSource + // to a falsy value on the very first stable iteration and silently disabling the + // Dependent::updateDependentSuggesters() call below. + if (!$versionSoftDeleted) { + $versionId = $result->id; + } } // pick the default branch when present, otherwise the first non-soft-deleted version @@ -358,7 +366,7 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep unset($existingVersions[$result->version]); } - if ($dependentSuggesterSource) { + if ($dependentSuggesterSource !== null) { $this->doctrine->getRepository(Dependent::class)->updateDependentSuggesters($package->getId(), $dependentSuggesterSource); } diff --git a/tests/Package/UpdaterTest.php b/tests/Package/UpdaterTest.php index e584c0874..33f7362de 100644 --- a/tests/Package/UpdaterTest.php +++ b/tests/Package/UpdaterTest.php @@ -15,8 +15,10 @@ use App\Audit\AuditRecordType; use App\Audit\VersionDeletionReason; use App\Entity\AuditRecord; +use App\Entity\Dependent; use App\Entity\Package; use App\Entity\PackageReadme; +use App\Entity\RequireLink; use App\Entity\Version; use App\Entity\VersionRepository; use App\Model\PackageManager; @@ -537,6 +539,35 @@ public function testDeleteBeforeWipesDevRowsButPreservesStableAndSoftDeletedStab self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked)); } + public function testDependentSuggesterSourceUpdatedForUnchangedStableVersion(): void + { + // Regression guard: a non-soft-deleted stable row that survives the immutability gate + // unchanged must still seed the dependent/suggester tables for the package — even though + // updateInformation returns VersionSkippedResult (no entity is loaded for the skipped row). + // Previously $versionId defaulted to false on the skip path, so the first such version + // pinned $dependentSuggesterSource to false and updateDependentSuggesters() never ran. + $em = self::getEM(); + $version = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $link = new RequireLink(); + $link->setVersion($version); + $link->setPackageName('composer/semver'); + $link->setPackageVersion('^3.2.0'); + $em->persist($link); + $em->flush(); + + $upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $this->repositoryMock = $this->createStub(VcsRepository::class); + $this->repositoryMock->method('getPackages')->willReturn([$upstream]); + $this->repositoryMock->method('getDriver')->willReturn($this->stableDriver()); + + $this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock); + + $em->clear(); + $dependents = $em->getRepository(Dependent::class)->findBy(['package' => $this->package->getId()]); + self::assertCount(1, $dependents, 'dependent row must be (re)inserted from the unchanged stable version'); + self::assertSame('composer/semver', $dependents[0]->getPackageName()); + } + public function testUpdateSourceDistUrlRewritesUrlsViaEntityAndKeepsRefsAndShasumIntact(): void { $em = self::getEM(); From b7547928599316faf6b766159cf519c2952b63d3 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 11:43:39 +0200 Subject: [PATCH 06/13] Update migrations/2026_05_version_immutability.sql Co-authored-by: Stephan --- migrations/2026_05_version_immutability.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/2026_05_version_immutability.sql b/migrations/2026_05_version_immutability.sql index 913d4e3ee..f35974508 100644 --- a/migrations/2026_05_version_immutability.sql +++ b/migrations/2026_05_version_immutability.sql @@ -1,7 +1,7 @@ -- Version immutability + soft-delete reasons -- -- Adds: --- deletionReason enum-string column (auto_missing | maintainer | admin) +-- deletionReason enum-string column (auto_missing | maintainer | admin | hidden) -- deletionReasonText optional human-readable reason for admin takedowns -- lastBlockedReference last attempted source/dist ref that was refused on a stable version -- index over (softDeletedAt, deletionReason) to keep the purge sweep fast From 45ac0e47db7dab75889d145863129fab36e1a9ac Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 13:48:04 +0200 Subject: [PATCH 07/13] Make sure hidden versions are not returned via the viewPackageVersionAction endpoint --- src/Audit/VersionDeletionReason.php | 9 --- src/Controller/PackageController.php | 26 ++++--- src/Package/Updater.php | 2 +- src/Security/Voter/PackageActions.php | 1 + src/Security/Voter/PackageVoter.php | 2 +- tests/Controller/PackageControllerTest.php | 85 ++++++++++++++++++++++ 6 files changed, 105 insertions(+), 20 deletions(-) diff --git a/src/Audit/VersionDeletionReason.php b/src/Audit/VersionDeletionReason.php index e4d13859e..c17b6acdb 100644 --- a/src/Audit/VersionDeletionReason.php +++ b/src/Audit/VersionDeletionReason.php @@ -77,13 +77,4 @@ public function isRecoverableByMaintainer(): bool self::DeletedByAdmin, self::Hidden => false, }; } - - /** - * Whether the soft-deleted version should still appear on the package page for the general - * public. Maintainers and admins see all soft-deleted versions regardless. - */ - public function isVisibleToPublic(): bool - { - return $this !== self::Hidden; - } } diff --git a/src/Controller/PackageController.php b/src/Controller/PackageController.php index 4f563bd3e..f3320f698 100644 --- a/src/Controller/PackageController.php +++ b/src/Controller/PackageController.php @@ -592,8 +592,8 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn /** @var Version[] $versions */ $versions = $package->getVersions()->toArray(); - if (!$this->isGranted(PackageActions::DeleteVersion->value, $package)) { - $versions = array_values(array_filter($versions, static fn (Version $v): bool => $v->getDeletionReason()?->isVisibleToPublic() ?? true)); + if (!$this->isGranted(PackageActions::ViewHiddenVersion->value, $package)) { + $versions = array_values(array_filter($versions, static fn (Version $v): bool => $v->getDeletionReason() !== VersionDeletionReason::Hidden)); } usort($versions, Package::class.'::sortVersions'); @@ -807,16 +807,24 @@ public function viewPackageVersionAction(Request $req, int $versionId): JsonResp $repo = $this->getEM()->getRepository(Version::class); try { - $html = $this->renderView( - 'package/version_details.html.twig', - ['version' => $version = $repo->getFullVersion($versionId)] - ); - } catch (NoResultException $e) { + $version = $repo->getFullVersion($versionId); + } catch (NoResultException) { return new JsonResponse(['status' => 'error', 'message' => 'The version could not be found, it may have been deleted in the meantime? Try reloading the page.'], 404); } - $resp = new JsonResponse(['content' => $html]); - if (!$version->isDevelopment()) { + if ($version->getDeletionReason() === VersionDeletionReason::Hidden && !$this->isGranted(PackageActions::ViewHiddenVersion->value, $version->getPackage())) { + return new JsonResponse(['status' => 'error', 'message' => 'The version could not be found, it may have been deleted in the meantime? Try reloading the page.'], 404); + } + + $resp = new JsonResponse([ + 'content' => $this->renderView( + 'package/version_details.html.twig', + ['version' => $version] + ) + ]); + // Hidden versions are only served to authorized viewers; shared-cache them and a CDN would + // hand the response to anyone hitting the same URL, bypassing the voter guard above. + if (!$version->isDevelopment() && $version->getDeletionReason() !== VersionDeletionReason::Hidden) { $resp->setSharedMaxAge(24 * 3600); $resp->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 'true'); } diff --git a/src/Package/Updater.php b/src/Package/Updater.php index 25cae2209..4ffed4d73 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -99,7 +99,7 @@ class Updater /** * Roll out new source/dist URLs across existing versions without changing references. - * Stable versions only accept the rewrite when every check in updateSourceDistUrl() passes + * Stable versions only accept the rewrite when every check in applySourceDistUrlRewrite() passes * (matching commit-hash refs, driver-confirmed dist URL, etc.); otherwise that version is skipped. */ public const UPDATE_SOURCE_DIST_URL = 1; diff --git a/src/Security/Voter/PackageActions.php b/src/Security/Voter/PackageActions.php index 51c3deb2a..1772b6a8c 100644 --- a/src/Security/Voter/PackageActions.php +++ b/src/Security/Voter/PackageActions.php @@ -21,6 +21,7 @@ enum PackageActions: string case Abandon = 'abandon'; case Delete = 'delete'; case DeleteVersion = 'delete_version'; + case ViewHiddenVersion = 'view_hidden_version'; case RecoverVersion = 'recover_version'; case AdminDeleteVersion = 'admin_delete_version'; case HideVersion = 'hide_version'; diff --git a/src/Security/Voter/PackageVoter.php b/src/Security/Voter/PackageVoter.php index 412878b34..b18e9b80b 100644 --- a/src/Security/Voter/PackageVoter.php +++ b/src/Security/Voter/PackageVoter.php @@ -52,7 +52,7 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter return match (PackageActions::from($attribute)) { PackageActions::Abandon => $this->canEdit($package, $user), PackageActions::Delete => $this->canDelete($package, $user), - PackageActions::DeleteVersion, PackageActions::RecoverVersion => $this->canDeleteVersion($package, $user), + PackageActions::ViewHiddenVersion, PackageActions::DeleteVersion, PackageActions::RecoverVersion => $this->canDeleteVersion($package, $user), PackageActions::AdminDeleteVersion, PackageActions::HideVersion => $this->canAdministerVersion(), PackageActions::Edit => $this->canEdit($package, $user), diff --git a/tests/Controller/PackageControllerTest.php b/tests/Controller/PackageControllerTest.php index 7a19d541d..f3ee23696 100644 --- a/tests/Controller/PackageControllerTest.php +++ b/tests/Controller/PackageControllerTest.php @@ -13,9 +13,12 @@ namespace App\Tests\Controller; use App\Audit\AuditRecordType; +use App\Audit\VersionDeletionReason; use App\Entity\Package; use App\Entity\User; +use App\Entity\Version; use App\Tests\IntegrationTestCase; +use Composer\Package\Version\VersionParser; use PHPUnit\Framework\Attributes\TestWith; class PackageControllerTest extends IntegrationTestCase @@ -230,4 +233,86 @@ public function testTransferPackageReturnsValidationError(?string $value, string $text = $elements->text(); $this->assertStringContainsStringIgnoringCase($message, $text); } + + #[TestWith([null, null, 200])] + #[TestWith([null, 'auto_missing', 200])] + #[TestWith([null, 'maintainer', 200])] + #[TestWith([null, 'admin', 200])] + #[TestWith([null, 'hidden', 404])] + #[TestWith(['maintainer', 'hidden', 200])] + #[TestWith(['admin', 'hidden', 200])] + public function testViewPackageVersionRespectsHiddenVisibility(?string $actor, ?string $reason, int $expectedStatus): void + { + $maintainer = self::createUser('owner', 'owner@example.org'); + $admin = self::createUser('admin', 'admin@example.org', roles: ['ROLE_ADMIN']); + $package = self::createPackage('test/pkg', 'https://example.com/test/pkg', maintainers: [$maintainer]); + $version = $this->createStableVersion($package, '1.0.0'); + if ($reason !== null) { + $version->setSoftDeletedAt(new \DateTimeImmutable()); + $version->setDeletionReason(VersionDeletionReason::from($reason)); + } + $this->store($maintainer, $admin, $package, $version); + + match ($actor) { + 'maintainer' => $this->client->loginUser($maintainer), + 'admin' => $this->client->loginUser($admin), + null => null, + }; + + $this->client->request('GET', '/versions/'.$version->getId().'.json'); + self::assertResponseStatusCodeSame($expectedStatus); + + if ($expectedStatus === 404) { + $payload = json_decode((string) $this->client->getResponse()->getContent(), true); + self::assertSame('error', $payload['status'] ?? null); + } + } + + public function testViewPackageVersionHiddenResponseIsNotSharedCached(): void + { + $maintainer = self::createUser('owner', 'owner@example.org'); + $package = self::createPackage('test/pkg', 'https://example.com/test/pkg', maintainers: [$maintainer]); + + $hidden = $this->createStableVersion($package, '1.0.0'); + $hidden->setSoftDeletedAt(new \DateTimeImmutable()); + $hidden->setDeletionReason(VersionDeletionReason::Hidden); + + $maintainerSoftDeleted = $this->createStableVersion($package, '1.1.0'); + $maintainerSoftDeleted->setSoftDeletedAt(new \DateTimeImmutable()); + $maintainerSoftDeleted->setDeletionReason(VersionDeletionReason::DeletedByMaintainer); + + $this->store($maintainer, $package, $hidden, $maintainerSoftDeleted); + + // Hidden, served to authorized maintainer -> must NOT be shared-cacheable. + $this->client->loginUser($maintainer); + $this->client->request('GET', '/versions/'.$hidden->getId().'.json'); + self::assertResponseStatusCodeSame(200); + $cacheControl = $this->client->getResponse()->headers->get('Cache-Control', ''); + self::assertStringNotContainsString('s-maxage', $cacheControl, 'Hidden version JSON must not advertise a shared-cache TTL'); + + // Non-Hidden soft-delete reason, served to anonymous -> keeps shared cache. Confirms the + // exemption above is Hidden-specific, not a blanket disable. + $this->client->restart(); + $this->client->request('GET', '/versions/'.$maintainerSoftDeleted->getId().'.json'); + self::assertResponseStatusCodeSame(200); + $cacheControl = $this->client->getResponse()->headers->get('Cache-Control', ''); + self::assertStringContainsString('s-maxage=86400', $cacheControl); + } + + private function createStableVersion(Package $package, string $version): Version + { + $v = new Version(); + $v->setName($package->getName()); + $v->setVersion($version); + $v->setNormalizedVersion(new VersionParser()->normalize($version)); + $v->setLicense(['MIT']); + $v->setAutoload([]); + $v->setDevelopment(false); + $v->setPackage($package); + $package->getVersions()->add($v); + $v->setReleasedAt(new \DateTimeImmutable()); + $v->setUpdatedAt(new \DateTimeImmutable()); + + return $v; + } } From 3613bc1017fc838f23bdba9277bd90997bc4521b Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 13:56:56 +0200 Subject: [PATCH 08/13] Clarify comments --- src/Package/Updater.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Package/Updater.php b/src/Package/Updater.php index 4ffed4d73..a776dee4d 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -393,14 +393,13 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep continue; } - // Dev rows are branch trackers and may be hard-purged after a 1-day grace period - // (or immediately if they're legacy v1-normalized dev-master/trunk/default rows that - // got re-created under a non-normalized name). Stable rows are immutable historical - // entries and stay soft-deleted forever — see VersionDeletionReason. if ( $isDev && ( + // Dev versions track branches and may be hard-purged after a 1-day grace period (null !== $version['softDeletedAt'] && new \DateTime($version['softDeletedAt']) < $deleteDate) + // ... or immediately if they're legacy v1-normalized dev-master/trunk/default rows that + // got re-created under a non-normalized name || ($version['normalizedVersion'] === '9999999-dev') ) ) { From 9dc3927f854737ebbaadc046efbd0ac93eedab68 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 15:24:28 +0200 Subject: [PATCH 09/13] Avoid updating frozen packages, just force a dump there if possible at all --- src/Entity/Package.php | 3 +++ src/Entity/VersionRepository.php | 9 +++++++-- src/Service/CdnClient.php | 16 +++------------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/Entity/Package.php b/src/Entity/Package.php index ebdc6fe89..fa75aefba 100644 --- a/src/Entity/Package.php +++ b/src/Entity/Package.php @@ -774,6 +774,9 @@ public function getSuspect(): ?string return $this->suspect; } + /** + * Freezing a package means it will not be updated anymore nor will its metadata be dumped + */ public function freeze(PackageFreezeReason $reason): void { $this->frozen = $reason; diff --git a/src/Entity/VersionRepository.php b/src/Entity/VersionRepository.php index 1880578f3..67590fa47 100644 --- a/src/Entity/VersionRepository.php +++ b/src/Entity/VersionRepository.php @@ -82,7 +82,7 @@ public function remove(Version $version, bool $createAuditRecord = true): void /** * Soft-delete a version with a reason. Schedules a fresh Updater run so dependents/suggesters - * and the V2 dump get recomputed without the version. + * and the V2 dump get recomputed without the version (frozen packages only get marked for dump). */ public function softDelete(Version $version, VersionDeletionReason $reason, ?string $reasonText, ?User $actor): void { @@ -94,7 +94,12 @@ public function softDelete(Version $version, VersionDeletionReason $reason, ?str $em->persist(AuditRecord::versionSoftDeleted($version, $reason, $reasonText, $actor)); - $this->scheduler->scheduleUpdate($version->getPackage(), 'version_soft_delete'); + if (!$version->getPackage()->isFrozen()) { + $this->scheduler->scheduleUpdate($version->getPackage(), 'version_recover'); + } else { + $version->getPackage()->setCrawledAt(new \DateTimeImmutable()); + $this->getEntityManager()->persist($version->getPackage()); + } } /** diff --git a/src/Service/CdnClient.php b/src/Service/CdnClient.php index 134fce49c..ef2f6716d 100644 --- a/src/Service/CdnClient.php +++ b/src/Service/CdnClient.php @@ -99,20 +99,10 @@ public function deleteMetadata(string $path): void ], ]); - if ($resp->getStatusCode() === 200) { - // purge the cache as well if the file was deleted - $resp = $this->httpClient->request('POST', 'https://api.bunny.net/purge?'.http_build_query(['url' => $this->metadataPublicEndpoint.$path, 'async' => 'true', 'exactPath' => 'true']), [ - 'headers' => [ - 'AccessKey' => $this->cdnApiKey, - ], - ]); - // wait for status code at least - $resp->getStatusCode(); + // purge the cache as well + $this->purgeMetadataCache($path); - return; - } - - if ($resp->getStatusCode() === 404) { + if (in_array($resp->getStatusCode(), [200, 404], true)) { return; } From 584b91856f7ab3892c1309d68c3d9d48e94e9f50 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 15:37:21 +0200 Subject: [PATCH 10/13] Remove dead code --- src/Entity/Version.php | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/Entity/Version.php b/src/Entity/Version.php index fd6c0b4a7..5cccbf64e 100644 --- a/src/Entity/Version.php +++ b/src/Entity/Version.php @@ -724,24 +724,6 @@ public function getLastBlockedReference(): ?string return $this->lastBlockedReference; } - /** - * Effective reference used for immutability identity: source.reference if present, - * else dist.reference, else null (no usable identity). - */ - public function getEffectiveReference(): ?string - { - $sourceRef = $this->source['reference'] ?? null; - if (\is_string($sourceRef) && $sourceRef !== '') { - return $sourceRef; - } - $distRef = $this->dist['reference'] ?? null; - if (\is_string($distRef) && $distRef !== '') { - return $distRef; - } - - return null; - } - /** * @return array */ From ca370a42d577ef1d90004bff88eb32df23638a5c Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 15:52:58 +0200 Subject: [PATCH 11/13] Minor cleanups --- src/Command/UpdatePackagesCommand.php | 6 +++--- src/Package/Updater.php | 15 ++++++++++++--- src/Service/UpdaterWorker.php | 4 ++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/Command/UpdatePackagesCommand.php b/src/Command/UpdatePackagesCommand.php index 167a18488..eb109b487 100644 --- a/src/Command/UpdatePackagesCommand.php +++ b/src/Command/UpdatePackagesCommand.php @@ -46,9 +46,9 @@ protected function configure(): void $this ->setName('packagist:update') ->setDefinition([ - new InputOption('force', null, InputOption::VALUE_NONE, 'Force a re-crawl of all packages, or if a package name is given forces an update of all versions'), + new InputOption('force', null, InputOption::VALUE_NONE, 'Force a re-crawl now and roll out source/dist URL changes across all versions (dev: full rebuild; stable: URL rewrite via the immutability gate, references stay frozen)'), new InputOption('delete-before', null, InputOption::VALUE_NONE, 'Force deletion of all dev versions before an update, stable versions remain'), - new InputOption('update-equal-refs', null, InputOption::VALUE_NONE, 'Force update of all versions even when they already exist'), + new InputOption('update-source-dist-url', null, InputOption::VALUE_NONE, 'Roll out new source/dist URLs across versions (stable rows: narrow URL rewrite via the strict gate; dev rows: full rebuild). Use after a repo URL change.'), new InputArgument('package', InputArgument::OPTIONAL, 'Package name to update'), ]) ->setDescription('Updates packages') @@ -100,7 +100,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($input->getOption('delete-before')) { $deleteBefore = true; } - if ($input->getOption('update-equal-refs')) { + if ($input->getOption('update-source-dist-url')) { $updateSourceDistUrl = true; } diff --git a/src/Package/Updater.php b/src/Package/Updater.php index a776dee4d..2a67d12b0 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -98,9 +98,18 @@ class Updater use \App\Util\DoctrineTrait; /** - * Roll out new source/dist URLs across existing versions without changing references. - * Stable versions only accept the rewrite when every check in applySourceDistUrlRewrite() passes - * (matching commit-hash refs, driver-confirmed dist URL, etc.); otherwise that version is skipped. + * Propagate a source/dist URL change across every version of a package. + * + * Stable versions: narrow rewrite via applySourceDistUrlRewrite() — only source.url and + * dist.url are touched, references/shasum/etc. are left frozen, and the row is skipped + * (with a warning) if any safety check fails. See applySourceDistUrlRewrite() for details. + * + * Dev versions: forces a full row rebuild even when the source.reference is unchanged, + * so the new URLs (and any other metadata) land on the next crawl. This is the + * "$flags & UPDATE_SOURCE_DIST_URL" branch in updateInformation(). + * + * Used after a repo rename/move (CLI --update-source-dist-url, the "Update All" UI button, + * or the post-rename detection path in ApiController::findGitHubPackagesByRepository()). */ public const UPDATE_SOURCE_DIST_URL = 1; public const DELETE_BEFORE = 2; diff --git a/src/Service/UpdaterWorker.php b/src/Service/UpdaterWorker.php index 5af446c87..b97ff41dc 100644 --- a/src/Service/UpdaterWorker.php +++ b/src/Service/UpdaterWorker.php @@ -197,11 +197,11 @@ public function process(Job $job, SignalHandler $signal): array $flags = 0; $useVersionCache = true; if (($job->getPayload()['update_source_dist_url'] ?? false) === true) { - $flags = Updater::UPDATE_SOURCE_DIST_URL; + $flags |= Updater::UPDATE_SOURCE_DIST_URL; $useVersionCache = false; } if ($job->getPayload()['delete_before'] === true) { - $flags = Updater::DELETE_BEFORE; + $flags |= Updater::DELETE_BEFORE; $useVersionCache = false; } if ($job->getPayload()['force_dump'] === true) { From 15bc74cae903fdae69ae9ca977070a483f4e9a49 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 17:05:40 +0200 Subject: [PATCH 12/13] Ensure we expand the first non-soft-deleted version if available --- src/Controller/PackageController.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Controller/PackageController.php b/src/Controller/PackageController.php index f3320f698..9af5c0fa0 100644 --- a/src/Controller/PackageController.php +++ b/src/Controller/PackageController.php @@ -613,11 +613,22 @@ public function viewPackageAction(Request $req, string $name, CsrfTokenManagerIn Assert::notNull($version); $expandedVersion = $version; + $softDeletedFallback = null; foreach ($versions as $candidate) { - if (!$candidate->isDevelopment()) { + if ($candidate->isDevelopment()) { + continue; + } + if ($candidate->getDeletionReason() === null) { $expandedVersion = $candidate; + $softDeletedFallback = null; break; } + if ($softDeletedFallback === null) { + $softDeletedFallback = $candidate; + } + } + if ($softDeletedFallback !== null && $expandedVersion === $version) { + $expandedVersion = $softDeletedFallback; } // load the expanded version fully to be able to display all info including tags From 4acb1696a92ea453a2a795868ea3dfa947cd23ff Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 4 Jun 2026 17:34:02 +0200 Subject: [PATCH 13/13] Fine tuning --- src/Command/CleanSpamPackagesCommand.php | 2 +- src/Entity/VersionRepository.php | 15 ++++++++- src/Model/PackageManager.php | 2 +- src/Package/Updater.php | 4 +++ tests/Entity/VersionRepositoryTest.php | 39 ++++++++++++++++++++-- tests/Package/UpdaterTest.php | 42 ++++++++++++++++++++++++ 6 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/Command/CleanSpamPackagesCommand.php b/src/Command/CleanSpamPackagesCommand.php index 9de69551f..b5150c33e 100644 --- a/src/Command/CleanSpamPackagesCommand.php +++ b/src/Command/CleanSpamPackagesCommand.php @@ -66,7 +66,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $output->write('.'); foreach ($package->getVersions() as $version) { - $versionRepo->remove($version, false); + $versionRepo->remove($version, false, allowStable: true); } $this->providerManager->deletePackage($package); diff --git a/src/Entity/VersionRepository.php b/src/Entity/VersionRepository.php index 67590fa47..3278f7d06 100644 --- a/src/Entity/VersionRepository.php +++ b/src/Entity/VersionRepository.php @@ -49,8 +49,21 @@ public function getEntityManager(): EntityManagerInterface return parent::getEntityManager(); } - public function remove(Version $version, bool $createAuditRecord = true): void + public function remove(Version $version, bool $createAuditRecord = true, bool $allowStable = false): void { + // Stable versions are immutable & permanent: their (package, version) slot must never be + // freed, or a later crawl could recreate it with different content and break the immutability + // guarantee that Composer lock-pinning, mirrors and scanners rely on. Pull a stable release + // with softDelete() instead. allowStable is only for whole-package deletion, where the entire + // package (and all its slots) is being removed. + if (!$allowStable && !$version->isDevelopment()) { + throw new \LogicException(sprintf( + 'Refusing to hard-delete stable version %s %s: stable releases are immutable; use softDelete().', + $version->getName(), + $version->getVersion() + )); + } + $em = $this->getEntityManager(); $package = $version->getPackage(); $package->getVersions()->removeElement($version); diff --git a/src/Model/PackageManager.php b/src/Model/PackageManager.php index 6d2f19967..1ddfbc11a 100644 --- a/src/Model/PackageManager.php +++ b/src/Model/PackageManager.php @@ -65,7 +65,7 @@ public function deletePackage(Package $package): void { $versionRepo = $this->doctrine->getRepository(Version::class); foreach ($package->getVersions() as $version) { - $versionRepo->remove($version, false); + $versionRepo->remove($version, false, allowStable: true); } if ($package->getAutoUpdated() === Package::AUTO_GITHUB_HOOK) { diff --git a/src/Package/Updater.php b/src/Package/Updater.php index 2a67d12b0..5c3598985 100644 --- a/src/Package/Updater.php +++ b/src/Package/Updater.php @@ -602,6 +602,10 @@ private function applySourceDistUrlRewrite(IOInterface $io, VersionRepository $v $oldSourceUrl = $oldSource['url'] ?? null; $oldDistUrlStored = $oldDist['url'] ?? null; + // dist.url was cross-checked against the driver above; source.url is taken from the incoming + // data without an equivalent driver check. That asymmetry is acceptable here: this path is + // operator-gated (UPDATE_SOURCE_DIST_URL only) and the reference hashes are pinned to the + // frozen snapshot, so only the URL can move, never the ref. $newSource = $oldSource; $newSource['url'] = $newSourceUrl; $newDist = $oldDist; diff --git a/tests/Entity/VersionRepositoryTest.php b/tests/Entity/VersionRepositoryTest.php index e3f56aea3..ca330456c 100644 --- a/tests/Entity/VersionRepositoryTest.php +++ b/tests/Entity/VersionRepositoryTest.php @@ -42,9 +42,11 @@ public function testRemoveVersionMarksForRemovalWithAuditRecord(bool $createAudi $version = new Version(); $version->setPackage($package); $version->setName($package->getName()); - $version->setVersion('1.0.0'); - $version->setNormalizedVersion('1.0.0.0'); - $version->setDevelopment(false); + // Dev versions are the ones legitimately hard-deleted (prune housekeeping, ClearVersions); + // stable versions are immutable and remove() refuses them (see testRemoveRefusesStableVersion). + $version->setVersion('dev-main'); + $version->setNormalizedVersion('dev-main'); + $version->setDevelopment(true); $version->setLicense([]); $version->setAutoload([]); $package->getVersions()->add($version); @@ -159,6 +161,37 @@ public function testGetVersionMetadataForUpdateIncludesNewProjection(): void self::assertSame('aabbccdd', $meta['2.0.0.0']['lastBlockedReference']); } + public function testRemoveRefusesStableVersion(): void + { + $version = $this->seedStableVersion('vendor/immutable', '2.0.0', '2.0.0.0'); + $versionId = $version->getId(); + + try { + $this->versionRepository->remove($version); + self::fail('Expected a LogicException when hard-deleting a stable version'); + } catch (\LogicException $e) { + self::assertStringContainsString('immutable', $e->getMessage()); + } + + self::getEM()->flush(); + self::getEM()->clear(); + self::assertNotNull($this->versionRepository->find($versionId), 'stable version must survive a refused hard-delete'); + } + + public function testRemoveAllowsStableVersionWithOptOut(): void + { + $version = $this->seedStableVersion('vendor/wholepkg', '2.0.0', '2.0.0.0'); + $versionId = $version->getId(); + + // allowStable is the whole-package-deletion escape hatch (PackageManager::deletePackage, + // CleanSpamPackagesCommand) where the entire package and all its slots are removed. + $this->versionRepository->remove($version, allowStable: true); + self::getEM()->flush(); + self::getEM()->clear(); + + self::assertNull($this->versionRepository->find($versionId), 'allowStable must permit hard-deleting a stable version'); + } + private function seedStableVersion(string $packageName, string $version, string $normalized): Version { $package = self::createPackage($packageName, 'https://github.com/'.$packageName); diff --git a/tests/Package/UpdaterTest.php b/tests/Package/UpdaterTest.php index 33f7362de..94e5112b6 100644 --- a/tests/Package/UpdaterTest.php +++ b/tests/Package/UpdaterTest.php @@ -684,6 +684,48 @@ public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithU self::assertNull($reloaded->getDeletionReason()); } + public function testStableVersionMissingUpstreamIsSoftDeletedNotRemovedThenRecovers(): void + { + $em = self::getEM(); + // Two active stable versions. Upstream will drop 1.0.0 but keep 1.1.0 around so the version + // list is never empty (and the recover SQL always has a non-empty id set to work with). + $gone = $this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $kept = $this->seedStableVersion($this->package, '1.1.0', '1.1.0.0', 'fedcba0987654321'); + $goneId = $gone->getId(); + $keptId = $kept->getId(); + + // Crawl 1: 1.0.0 has disappeared upstream. As an immutable stable version it must be + // soft-deleted (auto_missing), never hard-deleted — its (package, version) slot must persist. + $upstreamKept = $this->buildCompletePackage('test/pkg', '1.1.0', '1.1.0.0', 'fedcba0987654321'); + $repo1 = $this->createStub(VcsRepository::class); + $repo1->method('getPackages')->willReturn([$upstreamKept]); + $repo1->method('getDriver')->willReturn($this->stableDriver()); + $this->updater->update($this->ioMock, $this->config, $this->package, $repo1); + + $em->clear(); + $afterPrune = $em->getRepository(Version::class)->find($goneId); + self::assertNotNull($afterPrune, 'immutable stable version must never be hard-deleted when its tag disappears'); + self::assertNotNull($afterPrune->getSoftDeletedAt(), 'missing stable version must be soft-deleted'); + self::assertSame(VersionDeletionReason::AutoDeletedMissing, $afterPrune->getDeletionReason()); + self::assertNull($em->getRepository(Version::class)->find($keptId)?->getSoftDeletedAt(), 'still-present stable version stays active'); + + $package = $em->getRepository(Package::class)->find($this->package->getId()); + self::assertNotNull($package); + + // Crawl 2: the tag reappears at the same ref → the version auto-recovers. + $upstreamGone = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890'); + $repo2 = $this->createStub(VcsRepository::class); + $repo2->method('getPackages')->willReturn([$upstreamGone, $upstreamKept]); + $repo2->method('getDriver')->willReturn($this->stableDriver()); + $this->updater->update($this->ioMock, $this->config, $package, $repo2); + + $em->clear(); + $recovered = $em->getRepository(Version::class)->find($goneId); + self::assertNotNull($recovered); + self::assertNull($recovered->getSoftDeletedAt(), 'stable version must auto-recover when its tag reappears'); + self::assertNull($recovered->getDeletionReason()); + } + private function stableDriver(): VcsDriverInterface&Stub { $driver = $this->createStub(GitDriver::class);