From d252b72b05c8d2033733323527f2377a4ccfc851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 21 Oct 2026 15:34:38 -0300 Subject: [PATCH 1/3] fix(ci): distinguish pending status guard diagnostics --- .changelog/pending-status-diagnostics.md | 6 + .github/workflows/tests.yml | 20 ++- .../PendingTestStatusWorkflowTest.php | 120 ++++++++++++++++++ 3 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 .changelog/pending-status-diagnostics.md create mode 100644 tests/GitHubActions/PendingTestStatusWorkflowTest.php diff --git a/.changelog/pending-status-diagnostics.md b/.changelog/pending-status-diagnostics.md new file mode 100644 index 000000000..d0c5b1629 --- /dev/null +++ b/.changelog/pending-status-diagnostics.md @@ -0,0 +1,6 @@ +--- +category: fixed +type: patch +--- + +Distinguish pending test-status publisher identity, attempt and lifecycle validation failures without exposing response values or weakening fail-closed checks. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 03e69403d..e207cbd4a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -147,12 +147,20 @@ jobs: throw new RuntimeException('Duplicate PHP test matrix entry.'); } $run = githubApi(["repos/$repository/actions/runs/$runId"]); - if ((string) ($run['id'] ?? '') !== $runId - || ($run['repository']['full_name'] ?? null) !== $repository - || !is_int($run['run_attempt'] ?? null) - || (string) $run['run_attempt'] !== $attempt - || ($run['status'] ?? null) !== 'in_progress') { - throw new RuntimeException('The current workflow attempt is not running.'); + if ((string) ($run['id'] ?? '') !== $runId) { + throw new RuntimeException('Workflow run ID does not match the current run.'); + } + if (($run['repository']['full_name'] ?? null) !== $repository) { + throw new RuntimeException('Workflow repository does not match the current repository.'); + } + if (!is_int($run['run_attempt'] ?? null)) { + throw new RuntimeException('Workflow run attempt is not an integer.'); + } + if ((string) $run['run_attempt'] !== $attempt) { + throw new RuntimeException('Workflow run attempt does not match the current attempt.'); + } + if (($run['status'] ?? null) !== 'in_progress') { + throw new RuntimeException('Workflow run status is not in_progress.'); } // Validate the full matrix and current attempt before the first write. diff --git a/tests/GitHubActions/PendingTestStatusWorkflowTest.php b/tests/GitHubActions/PendingTestStatusWorkflowTest.php new file mode 100644 index 000000000..07a3ec1b4 --- /dev/null +++ b/tests/GitHubActions/PendingTestStatusWorkflowTest.php @@ -0,0 +1,120 @@ + + * @license https://opensource.org/licenses/MIT MIT License + * + * @see https://github.com/php-fast-forward/ + * @see https://github.com/php-fast-forward/dev-tools + * @see https://github.com/php-fast-forward/dev-tools/issues + * @see https://php-fast-forward.github.io/dev-tools/ + * @see https://datatracker.ietf.org/doc/html/rfc2119 + */ + +namespace FastForward\DevTools\Tests\GitHubActions; + +use PHPUnit\Framework\Attributes\CoversNothing; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\TestCase; +use Symfony\Component\Yaml\Yaml; +use function Safe\fclose; +use function Safe\fwrite; +use function Safe\proc_open; +use function Safe\stream_get_contents; + +#[CoversNothing] +final class PendingTestStatusWorkflowTest extends TestCase +{ + /** + * @return iterable, string}> + */ + public static function invalidRuns(): iterable + { + yield 'wrong ID' => [['id' => 8], 'Workflow run ID does not match the current run.']; + yield 'missing ID' => [['id' => null], 'Workflow run ID does not match the current run.']; + yield 'wrong repository' => [['repository' => ['full_name' => 'other/repo']], 'Workflow repository does not match the current repository.']; + yield 'missing repository' => [['repository' => null], 'Workflow repository does not match the current repository.']; + yield 'string attempt' => [['run_attempt' => '1'], 'Workflow run attempt is not an integer.']; + yield 'missing attempt' => [['run_attempt' => null], 'Workflow run attempt is not an integer.']; + yield 'different attempt' => [['run_attempt' => 2], 'Workflow run attempt does not match the current attempt.']; + foreach (['queued', 'completed', 'cancelled', null, "untrusted\nTOKEN_MARKER"] as $status) { + yield 'status ' . ($status ?? 'missing') => [['status' => $status], 'Workflow run status is not in_progress.']; + } + } + + /** + * @param array $overrides + */ + #[Test] + #[DataProvider('invalidRuns')] + public function pendingPublisherWillExplainEachRejectedRunWithoutWriting(array $overrides, string $diagnostic): void + { + [$code, $output] = $this->executePublisher($overrides); + + self::assertNotSame(0, $code); + self::assertStringContainsString($diagnostic, $output); + self::assertStringNotContainsString('STATUS_WRITE', $output); + self::assertStringNotContainsString('TOKEN_MARKER', $output); + } + + #[Test] + public function pendingPublisherWillKeepTheValidRunPublishingPath(): void + { + [$code, $output] = $this->executePublisher([]); + + self::assertSame(0, $code, $output); + self::assertSame(3, substr_count($output, 'STATUS_WRITE')); + } + + /** + * Execute the actual embedded program with only its GitHub transport substituted. + * + * @param array $overrides + * @return array{int, string} + */ + private function executePublisher(array $overrides): array + { + $workflow = Yaml::parseFile(__DIR__ . '/../../.github/workflows/tests.yml'); + $shell = $workflow['jobs']['publish_pending_statuses']['steps'][0]['run']; + self::assertIsString($shell); + $program = substr($shell, strpos($shell, ' 7, + 'repository' => ['full_name' => 'example/repo'], + 'run_attempt' => 1, + 'status' => 'in_progress', + ], $overrides); + $transport = 'function githubApi(array $arguments): array {' + . 'if ($arguments[0] === "--method") { echo "STATUS_WRITE\\n"; return []; }' + . 'return ' . var_export($run, true) . '; }' . "\n"; + $program = substr($program, 0, $start) . $transport . substr($program, $end); + $process = proc_open([PHP_BINARY], [['pipe', 'r'], ['pipe', 'w'], ['redirect', 1]], $pipes, null, array_merge(getenv(), [ + 'GITHUB_REPOSITORY' => 'example/repo', + 'GITHUB_RUN_ID' => '7', + 'GITHUB_RUN_ATTEMPT' => '1', + 'TARGET_SHA' => str_repeat('a', 40), + 'TARGET_URL' => 'https://github.com/example/repo/actions/runs/7', + 'TEST_MATRIX' => '{"php-version":["8.3","8.4","8.5"]}', + ])); + self::assertIsResource($process); + fwrite($pipes[0], $program); + fclose($pipes[0]); + $output = stream_get_contents($pipes[1]); + fclose($pipes[1]); + self::assertIsString($output); + + return [proc_close($process), $output]; + } +} From 607797279cc46e962f125e4cbac7c43cd1eca265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 21 Oct 2026 15:37:11 -0300 Subject: [PATCH 2/3] test: honor Safe proc_open string command contract --- tests/GitHubActions/PendingTestStatusWorkflowTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/GitHubActions/PendingTestStatusWorkflowTest.php b/tests/GitHubActions/PendingTestStatusWorkflowTest.php index 07a3ec1b4..06ba4363e 100644 --- a/tests/GitHubActions/PendingTestStatusWorkflowTest.php +++ b/tests/GitHubActions/PendingTestStatusWorkflowTest.php @@ -100,7 +100,7 @@ private function executePublisher(array $overrides): array . 'if ($arguments[0] === "--method") { echo "STATUS_WRITE\\n"; return []; }' . 'return ' . var_export($run, true) . '; }' . "\n"; $program = substr($program, 0, $start) . $transport . substr($program, $end); - $process = proc_open([PHP_BINARY], [['pipe', 'r'], ['pipe', 'w'], ['redirect', 1]], $pipes, null, array_merge(getenv(), [ + $process = proc_open(escapeshellarg(PHP_BINARY), [['pipe', 'r'], ['pipe', 'w'], ['redirect', 1]], $pipes, null, array_merge(getenv(), [ 'GITHUB_REPOSITORY' => 'example/repo', 'GITHUB_RUN_ID' => '7', 'GITHUB_RUN_ATTEMPT' => '1', From 8c181f9348ce97cb1daf42b4fb67fa5e2363a1d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 21 Oct 2026 18:55:18 -0300 Subject: [PATCH 3/3] test(ci): assert complete pending status requests --- .../GitHubActions/PendingTestStatusWorkflowTest.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/GitHubActions/PendingTestStatusWorkflowTest.php b/tests/GitHubActions/PendingTestStatusWorkflowTest.php index 06ba4363e..99ad67dce 100644 --- a/tests/GitHubActions/PendingTestStatusWorkflowTest.php +++ b/tests/GitHubActions/PendingTestStatusWorkflowTest.php @@ -71,6 +71,16 @@ public function pendingPublisherWillKeepTheValidRunPublishingPath(): void self::assertSame(0, $code, $output); self::assertSame(3, substr_count($output, 'STATUS_WRITE')); + foreach (['8.3', '8.4', '8.5'] as $version) { + $expected = [ + '--method', 'POST', 'repos/example/repo/statuses/' . str_repeat('a', 40), + '-f', 'state=pending', + '-f', "context=Run Tests ($version)", + '-f', "description=PHP $version matrix job is pending for attempt 1.", + '-f', 'target_url=https://github.com/example/repo/actions/runs/7', + ]; + self::assertStringContainsString(var_export($expected, true), $output); + } } /** @@ -97,7 +107,7 @@ private function executePublisher(array $overrides): array 'status' => 'in_progress', ], $overrides); $transport = 'function githubApi(array $arguments): array {' - . 'if ($arguments[0] === "--method") { echo "STATUS_WRITE\\n"; return []; }' + . 'if ($arguments[0] === "--method") { echo "STATUS_WRITE\\n" . var_export($arguments, true) . "\\n"; return []; }' . 'return ' . var_export($run, true) . '; }' . "\n"; $program = substr($program, 0, $start) . $transport . substr($program, $end); $process = proc_open(escapeshellarg(PHP_BINARY), [['pipe', 'r'], ['pipe', 'w'], ['redirect', 1]], $pipes, null, array_merge(getenv(), [