Merge pull request #677 from asgrim/676-fix-gh-api-break

676: fix GitHub Attestation bundle API BC break

Author: asgrim

composer.json

@@ -37,7 +37,7 @@
         "symfony/console": "^6.4.42",
         "symfony/event-dispatcher": "^6.4.37",
         "symfony/process": "^6.4.41",
-        "thephpf/attestation": "^0.0.5",
+        "thephpf/attestation": "^0.0.7",
         "webmozart/assert": "^1.12.1"
     },
     "require-dev": {

composer.lock

@@ -17,7 +17,7 @@
 use Php\Pie\SelfManage\Update\IsBrewInstallation;
 use Php\Pie\SelfManage\Update\ReleaseIsNewer;
 use Php\Pie\SelfManage\Update\ReleaseMetadata;
-use Php\Pie\SelfManage\Verify\FailedToVerifyRelease;
+use Php\Pie\SelfManage\Verify\RecoverFromFailedVerification;
 use Php\Pie\SelfManage\Verify\VerifyPieReleaseUsingAttestation;
 use Php\Pie\Settings;
 use Php\Pie\Util\Emoji;
@@ -27,6 +27,7 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\ConsoleOutputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Throwable;
 
@@ -178,17 +179,17 @@ public function execute(InputInterface $input, OutputInterface $output): int
 
         try {
             $verifyPiePhar->verify($latestRelease, $pharFilename, $this->io);
-        } catch (FailedToVerifyRelease $failedToVerifyRelease) {
-            $this->io->writeError(sprintf(
-                '<error>❌ Failed to verify the pie.phar release %s: %s</error>',
-                $latestRelease->tag,
-                $failedToVerifyRelease->getMessage(),
-            ));
-
-            $this->io->writeError('This means I could not verify that the PHAR we tried to update to was authentic, so I am aborting the self-update.');
-            unlink($pharFilename->filePath);
+        } catch (Throwable $verificationFailure) {
+            $this->getApplication()?->renderThrowable(
+                $verificationFailure,
+                $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output,
+            );
 
-            return Command::FAILURE;
+            if (! (new RecoverFromFailedVerification())($this->io, $latestRelease, $verificationFailure)) {
+                unlink($pharFilename->filePath);
+
+                return Command::FAILURE;
+            }
         }
 
         $pharContents = file_get_contents($pharFilename->filePath);

src/Command/SelfUpdateCommand.php

@@ -13,7 +13,6 @@
 use Php\Pie\File\FullPathToSelf;
 use Php\Pie\SelfManage\Update\FetchPieReleaseFromGitHub;
 use Php\Pie\SelfManage\Update\ReleaseMetadata;
-use Php\Pie\SelfManage\Verify\FailedToVerifyRelease;
 use Php\Pie\SelfManage\Verify\VerifyPieReleaseUsingAttestation;
 use Php\Pie\Util\Emoji;
 use Php\Pie\Util\PieVersion;
@@ -22,7 +21,9 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\ConsoleOutputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
+use Throwable;
 
 use function sprintf;
 
@@ -96,12 +97,16 @@ public function execute(InputInterface $input, OutputInterface $output): int
 
         try {
             $verifyPiePhar->verify($latestRelease, $pharFilename, $this->io);
-        } catch (FailedToVerifyRelease $failedToVerifyRelease) {
+        } catch (Throwable $verificationFailure) {
             $this->io->writeError(sprintf(
                 '<error>❌ Failed to verify that this PIE binary is the authentic release %s: %s</error>',
                 $latestRelease->tag,
-                $failedToVerifyRelease->getMessage(),
+                $verificationFailure->getMessage(),
             ));
+            $this->getApplication()?->renderThrowable(
+                $verificationFailure,
+                $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output,
+            );
 
             return Command::FAILURE;
         }

src/Command/SelfVerifyCommand.php

@@ -8,6 +8,7 @@
 use RuntimeException;
 use Symfony\Component\Process\Exception\ProcessFailedException;
 use ThePhpFoundation\Attestation\Verification\Exception\FailedToVerifyArtifact;
+use Throwable;
 
 use function sprintf;
 use function trim;
@@ -24,6 +25,18 @@ public static function fromNoOpenssl(): self
         return new self('Unable to verify without `gh` CLI tool, or openssl extension.');
     }
 
+    public static function fromUnexpectedException(Throwable $throwable): self
+    {
+        return new self(
+            sprintf(
+                'An unexpected error occurred while verifying the release (%s: %s)',
+                $throwable::class,
+                $throwable->getMessage(),
+            ),
+            previous: $throwable,
+        );
+    }
+
     public static function fromGhCliFailure(ReleaseMetadata $releaseMetadata, ProcessFailedException $processFailedException): self
     {
         return new self(

src/SelfManage/Verify/FailedToVerifyRelease.php

@@ -13,6 +13,7 @@
 use ThePhpFoundation\Attestation\FulcioSigstoreOidExtensions;
 use ThePhpFoundation\Attestation\Verification\Exception\FailedToVerifyArtifact;
 use ThePhpFoundation\Attestation\Verification\VerifyAttestation;
+use Throwable;
 
 use function sprintf;
 
@@ -70,6 +71,8 @@ public function verify(ReleaseMetadata $releaseMetadata, BinaryFile $pharFilenam
             );
         } catch (FailedToVerifyArtifact $failedToVerifyArtifact) {
             throw FailedToVerifyRelease::fromAttestationException($failedToVerifyArtifact);
+        } catch (Throwable $throwable) {
+            throw FailedToVerifyRelease::fromUnexpectedException($throwable);
         }
 
         $io->write(sprintf(

src/SelfManage/Verify/FallbackVerificationUsingOpenSsl.php

@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Php\Pie\SelfManage\Verify;
+
+use Composer\IO\IOInterface;
+use Php\Pie\SelfManage\Update\ReleaseMetadata;
+use Php\Pie\Util\Emoji;
+use Throwable;
+
+use function sprintf;
+
+/** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
+final class RecoverFromFailedVerification
+{
+    /**
+     * @return bool true if the caller should continue the update WITHOUT
+     *               verification (user opted in, at their own risk); false
+     *               if the caller should abort.
+     */
+    public function __invoke(IOInterface $io, ReleaseMetadata $releaseMetadata, Throwable $verificationFailure): bool
+    {
+        $io->writeError(sprintf(
+            '<error>%s Failed to verify the pie.phar release %s: %s</error>',
+            Emoji::CROSS,
+            $releaseMetadata->tag,
+            $verificationFailure->getMessage(),
+        ));
+        $io->writeError('<comment>This means I could not verify that the PHAR we tried to update to was authentic.</comment>');
+        $io->writeError(sprintf(
+            '<comment>You can manually download release %s yourself from: %s</comment>',
+            $releaseMetadata->tag,
+            $releaseMetadata->downloadUrl,
+        ));
+
+        if (! $io->isInteractive()) {
+            $io->writeError(sprintf('<warning>%s You are not running in interactive mode, so I am aborting the self-update.</warning>', Emoji::WARNING));
+
+            return false;
+        }
+
+        if (! $io->askConfirmation('<question>Would you like to continue the update <highlight>WITHOUT verification, at your own risk</highlight>? [y/N]</question>', false)) {
+            $io->writeError('<comment>Ok, aborting the self-update.</comment>');
+
+            return false;
+        }
+
+        $io->writeError(sprintf('<warning>%s Continuing the self-update WITHOUT verifying the release authenticity. This is at your own risk.</warning>', Emoji::WARNING));
+
+        return true;
+    }
+}

src/SelfManage/Verify/RecoverFromFailedVerification.php

@@ -16,7 +16,9 @@
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use ThePhpFoundation\Attestation\Verification\VerifyAttestation;
 use ThePhpFoundation\Attestation\Verification\VerifyAttestationWithOpenSsl;
+use Webmozart\Assert\InvalidArgumentException;
 
 use function assert;
 use function base64_encode;
@@ -145,16 +147,7 @@ private function mockAttestationResponse(string $digestInUrl, string $dsseEnvelo
         $url = self::TEST_GITHUB_URL . '/orgs/php/attestations/sha256:' . $digestInUrl . '?predicate_type=provenance';
         $this->httpDownloader->expects(self::once())
             ->method('get')
-            ->with(
-                $url,
-                [
-                    'retry-auth-failure' => true,
-                    'http' => [
-                        'method' => 'GET',
-                        'header' => [],
-                    ],
-                ],
-            )
+            ->with($url)
             ->willReturn(
                 new Response(
                     ['url' => $url],
@@ -369,4 +362,18 @@ public function testFailedToVerifyBecauseDigestNotFoundOnGitHub(): void
         $this->expectException(FailedToVerifyRelease::class);
         $this->verifier->verify($this->release, $this->downloadedPhar, $this->io);
     }
+
+    public function testUnexpectedThrowableFromVerifyAttestationIsWrappedInFailedToVerifyRelease(): void
+    {
+        $verifyAttestation = $this->createMock(VerifyAttestation::class);
+        $verifyAttestation->method('verify')
+            ->willThrowException(new InvalidArgumentException('Expected an array. Got: NULL'));
+
+        $verifier = new FallbackVerificationUsingOpenSsl($verifyAttestation, $this->fetchPieRelease);
+
+        $this->expectException(FailedToVerifyRelease::class);
+        $this->expectExceptionMessageMatches('/Expected an array\. Got: NULL/');
+
+        $verifier->verify($this->release, $this->downloadedPhar, $this->io);
+    }
 }

test/unit/SelfManage/Verify/FallbackVerificationUsingOpenSslTest.php

@@ -0,0 +1,69 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Php\PieUnitTest\SelfManage\Verify;
+
+use Composer\IO\BufferIO;
+use Php\Pie\SelfManage\Update\ReleaseMetadata;
+use Php\Pie\SelfManage\Verify\RecoverFromFailedVerification;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+use RuntimeException;
+
+#[CoversClass(RecoverFromFailedVerification::class)]
+final class RecoverFromFailedVerificationTest extends TestCase
+{
+    private const DOWNLOAD_URL = 'https://example.localhost/pie.phar';
+
+    private ReleaseMetadata $release;
+    private RuntimeException $verificationFailure;
+    private RecoverFromFailedVerification $recover;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->release             = new ReleaseMetadata('1.2.3', self::DOWNLOAD_URL);
+        $this->verificationFailure = new RuntimeException('some failure');
+        $this->recover             = new RecoverFromFailedVerification();
+    }
+
+    public function testAbortsWithoutPromptingWhenNonInteractive(): void
+    {
+        $io = new BufferIO();
+
+        $result = ($this->recover)($io, $this->release, $this->verificationFailure);
+
+        self::assertFalse($result);
+        $output = $io->getOutput();
+        self::assertStringContainsString(self::DOWNLOAD_URL, $output);
+        self::assertStringContainsString('not running in interactive mode', $output);
+    }
+
+    public function testAbortsWhenUserDeclinesConfirmation(): void
+    {
+        $io = new BufferIO();
+        $io->setUserInputs(['n']);
+
+        $result = ($this->recover)($io, $this->release, $this->verificationFailure);
+
+        self::assertFalse($result);
+        $output = $io->getOutput();
+        self::assertStringContainsString(self::DOWNLOAD_URL, $output);
+        self::assertStringContainsString('aborting', $output);
+    }
+
+    public function testContinuesWhenUserConfirms(): void
+    {
+        $io = new BufferIO();
+        $io->setUserInputs(['y']);
+
+        $result = ($this->recover)($io, $this->release, $this->verificationFailure);
+
+        self::assertTrue($result);
+        $output = $io->getOutput();
+        self::assertStringContainsString(self::DOWNLOAD_URL, $output);
+        self::assertStringContainsString('at your own risk', $output);
+    }
+}