diff --git a/.changelog/optional-analysis-summaries.md b/.changelog/optional-analysis-summaries.md new file mode 100644 index 0000000..7bd1375 --- /dev/null +++ b/.changelog/optional-analysis-summaries.md @@ -0,0 +1,10 @@ +--- +category: added +type: minor +--- + +Add explicitly enabled Magic Numbers and Rector workflow analyses with bounded native-report classification, advisory or blocking findings policies, always-visible summaries and failing operational errors. Expose native Rector JSON preview without granting semantic apply authority. + +Declare the project's consumer reusable call in a PR-only workflow while preserving push and manual project-quality testing. + +Keep Magic Numbers file selection independent from Rector's configured whole-project analysis. diff --git a/.github/OptionalAnalysis/AnalysisResult.php b/.github/OptionalAnalysis/AnalysisResult.php new file mode 100644 index 0000000..8d91034 --- /dev/null +++ b/.github/OptionalAnalysis/AnalysisResult.php @@ -0,0 +1,50 @@ + $diagnostics Native locations/messages. + * @param string $details Bounded later by the renderer, and never interpreted as Markdown or workflow commands. + * @param int|null $nativeExit Captured Code Standards invocation status; null if completion is unavailable. + */ + public function __construct( + public string $tool, + public string $status, + public ?int $selectedCount = null, + public ?int $findingCount = null, + public array $diagnostics = [], + public string $details = '', + public ?int $nativeExit = null, + ) { + if (!in_array($tool, ['Magic Numbers', 'Rector'], true) + || !in_array($status, ['disabled', 'not-executed', 'clean', 'findings', 'operational-error'], true) + || (null !== $selectedCount && $selectedCount < 0) + || (null !== $findingCount && $findingCount < 0) + || (null !== $nativeExit && $nativeExit < 0)) { + throw new InvalidArgumentException('Invalid optional-analysis result.'); + } + } + + /** Only verified findings may become advisory; errors stay failing under either policy. */ + public function blocks(string $policy): bool + { + if (!in_array($policy, ['report', 'fail'], true)) { + throw new InvalidArgumentException('Findings policy must be report or fail.'); + } + + return 'operational-error' === $this->status || ('findings' === $this->status && 'fail' === $policy); + } +} diff --git a/.github/OptionalAnalysis/BoundedProcess.php b/.github/OptionalAnalysis/BoundedProcess.php new file mode 100644 index 0000000..d07e71f --- /dev/null +++ b/.github/OptionalAnalysis/BoundedProcess.php @@ -0,0 +1,227 @@ + $arguments Installed Code Standards entrypoint and explicitly validated options. + */ + public function capture(array $arguments, string $directory, float $timeout = self::TIMEOUT_SECONDS): ProcessResult + { + $this->assertPrerequisites($timeout); + $deadline = hrtime(true) + (int) ($timeout * 1_000_000_000); + $environment = getenv(); + foreach (self::PRIVATE_ENVIRONMENT_FIELDS as $name) { + unset($environment[$name]); + } + $temporary = $this->temporary(); + $environment['TMPDIR'] = $temporary; + try { + $process = @proc_open([ + '/usr/bin/setsid', '--wait', PHP_BINARY, __DIR__ . '/launch.php', ...$arguments, + ], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w'], 3 => ['pipe', 'w']], $pipes, $directory, $environment); + } catch (Throwable $exception) { + $this->removeTemporary($temporary); + + throw $exception; + } + if (!is_resource($process)) { + $this->removeTemporary($temporary); + throw new RuntimeException('Cannot start the owned optional-analyzer process.'); + } + $pid = proc_get_status($process)['pid']; + $confirmed = false; + try { + $this->confirmGroup($pipes[3], $pid, min($deadline, hrtime(true) + 2_000_000_000)); + $confirmed = true; + if (1 !== fwrite($pipes[0], 'S')) { + throw new RuntimeException('Cannot authorize the owned analyzer to start.'); + } + fclose($pipes[0]); + fclose($pipes[3]); + unset($pipes[0], $pipes[3]); + [$stdout, $stderr, $status] = $this->read($process, $pipes, $deadline); + } finally { + if ($confirmed) { + // This group was created and verified before any consumer configuration could execute. + @posix_kill(-$pid, 15); + @posix_kill(-$pid, 9); + } else { + // Before the start handshake, only the trusted launcher can exist. + @proc_terminate($process, 9); + } + foreach ($pipes as $pipe) { + if (is_resource($pipe)) { + fclose($pipe); + } + } + $closed = proc_close($process); + $this->removeTemporary($temporary); + } + $status ??= $closed; + if ($status < 0) { + throw new RuntimeException('Optional analyzer completion status is unavailable.'); + } + + return new ProcessResult($status, $stdout, $stderr); + } + + /** Missing hosted-Linux isolation support fails before launching anything; there is no weaker fallback. */ + private function assertPrerequisites(float $timeout): void + { + if (!is_finite($timeout) || $timeout <= 0 || $timeout > self::TIMEOUT_SECONDS) { + throw new InvalidArgumentException('Optional analyzer timeout must be positive, finite and at most 300 seconds.'); + } + if ('Linux' !== PHP_OS_FAMILY + || !is_executable('/usr/bin/setsid') + || !function_exists('posix_getpgid') + || !function_exists('posix_getsid') + || !function_exists('posix_kill')) { + throw new RuntimeException('Optional workflow execution requires hosted Linux, setsid and PHP POSIX process-group support.'); + } + } + + /** The trusted launcher blocks before payload execution until its own PID/session/group have been checked. */ + private function confirmGroup($control, int $pid, int $deadline): void + { + stream_set_blocking($control, false); + $identity = ''; + while (!str_contains($identity, "\n")) { + if (hrtime(true) >= $deadline) { + throw new RuntimeException('Owned analyzer session handshake timed out.'); + } + $bytes = fread($control, 32); + if (false === $bytes || ('' === $bytes && feof($control))) { + throw new RuntimeException('Owned analyzer session handshake failed.'); + } + $identity .= $bytes; + if (strlen($identity) > 32) { + throw new RuntimeException('Owned analyzer session identity is invalid.'); + } + usleep(1000); + } + if ($identity !== $pid . "\n" || @posix_getpgid($pid) !== $pid || @posix_getsid($pid) !== $pid) { + throw new RuntimeException('Owned analyzer process group could not be confirmed.'); + } + } + + /** + * Reads fixed chunks fairly from both channels and checks the deadline between reads. + * + * @param resource $process The exclusively owned trusted launcher resource. + * @param array $pipes Nonblocking stdout/stderr pipes, closed as EOF is observed. + * @return array{string, string, int|null} Exact bounded channels and the observed completion status. + */ + private function read($process, array &$pipes, int $deadline): array + { + foreach ($pipes as $pipe) { + stream_set_blocking($pipe, false); + } + $captured = [1 => '', 2 => '']; + $status = null; + do { + if (hrtime(true) >= $deadline) { + throw new RuntimeException('Optional analyzer exceeded its total invocation timeout; no clean result is available.'); + } + $ready = array_values($pipes); + $write = $except = []; + if ([] !== $ready && false === @stream_select($ready, $write, $except, 0, 100000)) { + throw new RuntimeException('Cannot read optional-analyzer channels.'); + } + foreach ($ready as $pipe) { + $channel = array_search($pipe, $pipes, true); + if (false === $channel) { + throw new RuntimeException('Optional analyzer channel identity is unavailable.'); + } + $limit = 1 === $channel ? self::REPORT_BYTES : self::STDERR_BYTES; + $bytes = fread($pipe, min(self::CHUNK_BYTES, $limit - strlen($captured[$channel]) + 1)); + if (false === $bytes || strlen($captured[$channel]) + strlen($bytes) > $limit) { + throw new RuntimeException('Optional analyzer output exceeded its capture limit; the incomplete report is not classified.'); + } + $captured[$channel] .= $bytes; + if (feof($pipe)) { + fclose($pipe); + unset($pipes[$channel]); + } + } + $state = proc_get_status($process); + if (!$state['running']) { + $status = $state['signaled'] ? 128 + $state['termsig'] : ($state['exitcode'] >= 0 ? $state['exitcode'] : $status); + } + if ([] === $pipes && $state['running']) { + usleep(10000); + } + } while ([] !== $pipes || $state['running']); + + return [$captured[1], $captured[2], $status]; + } + + /** Native preflight/cache temporary files belong to this invocation, never the consumer or a shared cache. */ + private function temporary(): string + { + $base = realpath(getenv('RUNNER_TEMP') ?: sys_get_temp_dir()); + if (false === $base) { + throw new RuntimeException('Runner temporary directory is unavailable.'); + } + $path = $base . '/standards-optional-' . bin2hex(random_bytes(12)); + if (!mkdir($path, 0700)) { + throw new RuntimeException('Cannot create owned analyzer temporary directory.'); + } + + return $path; + } + + /** Traverses only the freshly owned directory and never follows links created by executable configuration. */ + private function removeTemporary(string $directory, int &$remaining = 4096): void + { + if (is_link($directory) || is_file($directory)) { + if (!unlink($directory)) { + throw new RuntimeException('Cannot remove the owned temporary path replacement.'); + } + + return; + } + if (!is_dir($directory)) { + return; + } + foreach (new \FilesystemIterator($directory, \FilesystemIterator::SKIP_DOTS) as $file) { + if (--$remaining < 0) { + throw new RuntimeException('Owned analyzer temporary cleanup exceeded its entry bound; runner teardown must remove the remainder.'); + } + if ($file->isLink() || !$file->isDir()) { + if (!unlink($file->getPathname())) { + throw new RuntimeException('Cannot remove an owned analyzer temporary entry.'); + } + } else { + $this->removeTemporary($file->getPathname(), $remaining); + } + } + if (!rmdir($directory)) { + throw new RuntimeException('Cannot remove the owned analyzer temporary directory.'); + } + } +} diff --git a/.github/OptionalAnalysis/NativeReportParser.php b/.github/OptionalAnalysis/NativeReportParser.php new file mode 100644 index 0000000..6ecd1b8 --- /dev/null +++ b/.github/OptionalAnalysis/NativeReportParser.php @@ -0,0 +1,223 @@ + $selectedFiles Exact absolute files from the existing Git/PHP selector. + * @throws InvalidArgumentException When output, counts, paths or status contradict the qualified schema. + */ + public function magicNumbers(ProcessResult $process, array $selectedFiles): AnalysisResult + { + $root = $this->xml($process->stdout); + $reportedFiles = $this->children($root, 'files'); + if (1 !== count($reportedFiles)) { + throw new InvalidArgumentException('PHPMND XML must contain exactly one files element.'); + } + $allowed = array_fill_keys($selectedFiles, true); + $diagnostics = []; + $seen = []; + foreach ($this->children($reportedFiles[0], 'file') as $file) { + $path = $file->getAttribute('path'); + if (!isset($allowed[$path]) || isset($seen[$path])) { + throw new InvalidArgumentException('PHPMND reported an unexpected or repeated selected path.'); + } + $seen[$path] = true; + $entries = $this->children($file, 'entry'); + if ([] === $entries || count($entries) !== $this->integer($file->getAttribute('errors'))) { + throw new InvalidArgumentException('PHPMND file finding count is inconsistent.'); + } + foreach ($entries as $entry) { + $line = $this->integer($entry->getAttribute('line')); + if ($line < 1) { + throw new InvalidArgumentException('PHPMND finding line must be positive.'); + } + $diagnostics[] = ['file' => $path, 'line' => $line, 'message' => 'Numeric literal requires review.']; + } + } + $count = $this->integer($root->getAttribute('errorCount')); + if ($count !== count($diagnostics) || count($seen) !== $this->integer($root->getAttribute('fileCount'))) { + throw new InvalidArgumentException('PHPMND report totals are inconsistent.'); + } + if ((0 === $count && 0 !== $process->exitCode) || (0 !== $count && 1 !== $process->exitCode)) { + throw new InvalidArgumentException('PHPMND exit contradicts its complete native report.'); + } + + return new AnalysisResult('Magic Numbers', 0 === $count ? 'clean' : 'findings', count($selectedFiles), $count, $diagnostics, $process->stderr, $process->exitCode); + } + + /** Native Rector errors remain blocking even when the same report also proposes changes. */ + public function rector(ProcessResult $process): AnalysisResult + { + if (strlen($process->stdout) > BoundedProcess::REPORT_BYTES) { + throw new InvalidArgumentException('Rector native report exceeds the supported size.'); + } + try { + $report = json_decode($process->stdout, true, flags: JSON_THROW_ON_ERROR); + } catch (JsonException $exception) { + throw new InvalidArgumentException('Rector did not return a complete native JSON document.', previous: $exception); + } + if (!is_array($report) || !isset($report['totals']) || !is_array($report['totals'])) { + throw new InvalidArgumentException('Rector JSON totals are unavailable.'); + } + $changes = $this->jsonInteger($report['totals']['changed_files'] ?? null); + $errorCount = $this->jsonInteger($report['totals']['errors'] ?? null); + $errors = $this->list($report['errors'] ?? []); + if ($errorCount !== count($errors)) { + throw new InvalidArgumentException('Rector error total is inconsistent.'); + } + $files = $this->list($report['changed_files'] ?? []); + if (count($files) !== $changes || count(array_filter($files, static fn(mixed $file): bool => is_string($file) && '' !== $file)) !== count($files)) { + throw new InvalidArgumentException('Rector changed-file list is inconsistent.'); + } + $diagnostics = [...$this->rectorErrors($errors), ...$this->rectorChanges($this->list($report['file_diffs'] ?? []), $files)]; + if (0 !== $errorCount) { + if (1 !== $process->exitCode) { + throw new InvalidArgumentException('Rector error report contradicts its exit.'); + } + + return new AnalysisResult('Rector', 'operational-error', findingCount: $changes, diagnostics: $diagnostics, details: $errorCount . " native processing error(s).\n" . $process->stderr, nativeExit: $process->exitCode); + } + if ((0 === $changes && 0 !== $process->exitCode) || (0 !== $changes && 2 !== $process->exitCode)) { + throw new InvalidArgumentException('Rector preview exit contradicts its native totals.'); + } + return new AnalysisResult('Rector', 0 === $changes ? 'clean' : 'findings', findingCount: $changes, diagnostics: $diagnostics, details: "Scope is the explicit native configuration, not the formatter changed-file list.\n" . $process->stderr, nativeExit: $process->exitCode); + } + + /** Parses without external resolution/substitution, then rejects DTDs; CDATA remains ordinary source text. */ + private function xml(string $output): DOMElement + { + if (strlen($output) > BoundedProcess::REPORT_BYTES || !extension_loaded('dom') || 1 !== preg_match('//u', $output) || str_contains($output, "\0")) { + throw new InvalidArgumentException('PHPMND requires UTF-8 native XML without DTD/entity declarations and an available DOM extension.'); + } + $previous = libxml_use_internal_errors(true); + try { + $document = new DOMDocument(); + $document->resolveExternals = false; + $document->substituteEntities = false; + $document->validateOnParse = false; + if (!$document->loadXML($output, LIBXML_NONET) || null !== $document->doctype || 'phpmnd' !== $document->documentElement?->tagName) { + throw new InvalidArgumentException('PHPMND did not return a complete native XML document.'); + } + if ('' === $document->documentElement->getAttribute('version')) { + throw new InvalidArgumentException('PHPMND XML version is unavailable.'); + } + + return $document->documentElement; + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previous); + } + } + + /** @return list Expected structural elements only; whitespace text/comments are allowed, snippets remain opaque. */ + private function children(DOMElement $parent, string $name): array + { + $elements = []; + foreach ($parent->childNodes as $child) { + if ($child instanceof DOMElement) { + if ($name !== $child->tagName) { + throw new InvalidArgumentException('Unexpected PHPMND XML structure.'); + } + $elements[] = $child; + + continue; + } + if (XML_COMMENT_NODE === $child->nodeType + || (XML_TEXT_NODE === $child->nodeType && '' === trim($child->textContent))) { + continue; + } + + throw new InvalidArgumentException('Unexpected PHPMND XML structural node.'); + } + + return $elements; + } + + /** Exact nonnegative decimal strings avoid coercion, overflow and misleading native counts. */ + private function integer(string $value): int + { + $integer = (int) $value; + if (1 !== preg_match('/\A(?:0|[1-9][0-9]*)\z/D', $value) || (string) $integer !== $value) { + throw new InvalidArgumentException('Native report count is invalid.'); + } + + return $integer; + } + + /** Native JSON counts must be integers rather than coerced strings or floating point values. */ + private function jsonInteger(mixed $value): int + { + if (!is_int($value) || $value < 0) { + throw new InvalidArgumentException('Rector JSON count is invalid.'); + } + + return $value; + } + + /** @return list Native ordered report entries, not an arbitrary keyed JSON object. */ + private function list(mixed $value): array + { + if (!is_array($value) || !array_is_list($value)) { + throw new InvalidArgumentException('Rector JSON entries must be a list.'); + } + + return $value; + } + + /** + * Retains actual native error locations; an unavailable path is not replaced with a guessed source file. + * + * @param list $errors Native error records. + * @return list + */ + private function rectorErrors(array $errors): array + { + $diagnostics = []; + foreach ($errors as $error) { + if (!is_array($error) + || (isset($error['file']) && !is_string($error['file'])) + || !is_string($error['message'] ?? null) + || (isset($error['line']) && (!is_int($error['line']) || $error['line'] < 1))) { + throw new InvalidArgumentException('Rector JSON error location is invalid.'); + } + $diagnostics[] = ['file' => $error['file'] ?? '[unavailable]', 'line' => $error['line'] ?? null, 'message' => $error['message']]; + } + + return $diagnostics; + } + + /** + * Lists native changed files without interpreting diff text as line or AST identity. + * + * @param list $changes Optional native file-diff records, validated without displaying source. + * @param list $files The native changed-file list corresponding to the total. + * @return list + */ + private function rectorChanges(array $changes, array $files): array + { + foreach ($changes as $change) { + if (!is_array($change) || !is_string($change['file'] ?? null) || !in_array($change['file'], $files, true) || !is_string($change['diff'] ?? null)) { + throw new InvalidArgumentException('Rector JSON change location is invalid.'); + } + } + $diagnostics = []; + foreach ($files as $file) { + $diagnostics[] = ['file' => $file, 'line' => null, 'message' => 'Semantic changes proposed; the native JSON does not provide a line field.']; + } + + return $diagnostics; + } +} diff --git a/.github/OptionalAnalysis/OptionalAnalysisRunner.php b/.github/OptionalAnalysis/OptionalAnalysisRunner.php new file mode 100644 index 0000000..b13997f --- /dev/null +++ b/.github/OptionalAnalysis/OptionalAnalysisRunner.php @@ -0,0 +1,111 @@ + A result exists for both tools, including disabled/error paths. */ + public function run(WorkflowInput $input): array + { + $magic = new AnalysisResult('Magic Numbers', 'disabled', details: 'Not enabled; no analyzer executed.'); + $rector = new AnalysisResult('Rector', 'disabled', details: 'Not enabled; no analyzer executed.'); + if (!$input->magicNumbers && !$input->rector) { + return [$magic, $rector]; + } + try { + $directory = realpath($input->directory); + if (false === $directory || !is_dir($directory)) { + throw new InvalidArgumentException('Consumer directory is unavailable.'); + } + } catch (Throwable $exception) { + return [ + $input->magicNumbers ? $this->failure('Magic Numbers', $exception) : $magic, + $input->rector ? $this->failure('Rector', $exception) : $rector, + ]; + } + if ($input->magicNumbers) { + $magic = $this->magicNumbers($input, $directory); + } + if ($input->rector) { + $rector = $this->rector($input, $directory); + } + + return [$magic, $rector]; + } + + /** Selects only Magic Numbers' eligible paths using the exact-PR/ignore contract. */ + private function magicNumbers(WorkflowInput $input, string $directory): AnalysisResult + { + $files = null; + $result = null; + try { + $paths = $this->selection->select($directory, $input->base, $input->head); + $files = $this->phpFiles->filter($directory, $paths); + if ([] === $files) { + return new AnalysisResult('Magic Numbers', 'not-executed', 0, details: 'No eligible changed PHP files; the engine was not executed.'); + } + $result = $this->process->capture([ + PHP_BINARY, '-d', 'memory_limit=256M', $this->root . '/bin/code-standards', 'magic-numbers', + '--directory=' . $directory, '--base=' . $input->base, '--head=' . $input->head, '--format=xml', + ], $directory); + + return $this->reports->magicNumbers($result, $files); + } catch (Throwable $exception) { + return $this->failure('Magic Numbers', $exception, null === $files ? null : count($files), $result); + } + } + + /** Rector retains explicitly configured whole-project paths and always runs preview, never apply. */ + private function rector(WorkflowInput $input, string $directory): AnalysisResult + { + $result = null; + try { + if ('' === $input->rectorConfig || str_starts_with($input->rectorConfig, '/')) { + throw new InvalidArgumentException('Enabled Rector requires a consumer-relative rector-config input.'); + } + $scope = $this->projects->resolve(true, null, null, $directory, $input->rectorConfig); + if (!str_starts_with($scope->config, $directory . '/')) { + throw new InvalidArgumentException('Workflow Rector configuration must resolve inside the consumer checkout.'); + } + $result = $this->process->capture([ + PHP_BINARY, '-d', 'memory_limit=256M', $this->root . '/bin/code-standards', 'refactor', + '--directory=' . $directory, '--config=' . $scope->config, '--format=json', + ], $directory); + + return $this->reports->rector($result); + } catch (Throwable $exception) { + return $this->failure('Rector', $exception, process: $result); + } + } + + /** Exceptions never turn into advisory findings or an unavailable zero count. */ + private function failure(string $tool, Throwable $exception, ?int $selected = null, ?ProcessResult $process = null): AnalysisResult + { + $details = $exception->getMessage(); + if (null !== $process && '' !== $process->stderr) { + $details .= "\nNative stderr: " . $process->stderr; + } + + return new AnalysisResult($tool, 'operational-error', $selected, details: $details, nativeExit: $process?->exitCode); + } +} diff --git a/.github/OptionalAnalysis/SummaryRenderer.php b/.github/OptionalAnalysis/SummaryRenderer.php new file mode 100644 index 0000000..de19e93 --- /dev/null +++ b/.github/OptionalAnalysis/SummaryRenderer.php @@ -0,0 +1,57 @@ + $results Known tools/results, independent of findings severity. */ + public function render(array $results): string + { + $summary = "## Optional Code Standards analyses\n\n"; + $summary .= "| Tool | Status | Selected PHP files | Native count | Invocation exit |\n| --- | --- | ---: | ---: | ---: |\n"; + foreach ($results as $result) { + $label = 'Rector' === $result->tool ? 'changed files' : 'findings'; + $summary .= '| ' . $result->tool . ' | ' . $result->status . ' | ' . ($result->selectedCount ?? 'unknown/native scope') . ' | ' . ($result->findingCount ?? 'unavailable') . ' ' . $label . ' | ' . ($result->nativeExit ?? 'not available') . " |\n"; + } + $summary .= "\n"; + foreach ($results as $result) { + $summary .= '### ' . $result->tool . "\n\n"; + $summary .= "
\n";
+            $lines = '' === $result->details ? [] : [$result->details];
+            foreach (array_slice($result->diagnostics, 0, self::MAX_DIAGNOSTICS) as $diagnostic) {
+                $lines[] = $diagnostic['file'] . ':' . ($diagnostic['line'] ?? '?') . ' - ' . $diagnostic['message'];
+            }
+            foreach ($lines as $line) {
+                $escaped = $this->escape($line) . "\n";
+                if (strlen($summary) + strlen($escaped) > self::MAX_BYTES - 512) {
+                    return $summary . "
\n\nAdditional diagnostics omitted to keep the summary bounded. Native totals above are unchanged.\n"; + } + $summary .= $escaped; + } + $summary .= "\n\n"; + if (count($result->diagnostics) > self::MAX_DIAGNOSTICS) { + $summary .= "Additional diagnostics omitted; native totals are not truncated.\n\n"; + } + } + + return $summary; + } + + /** Control bytes remain visible and escaped; untrusted closing tags cannot end the preformatted block. */ + private function escape(string $text): string + { + $truncated = strlen($text) > self::MAX_FIELD_BYTES; + $text = substr($text, 0, self::MAX_FIELD_BYTES); + $quoted = json_encode($text, JSON_INVALID_UTF8_SUBSTITUTE | JSON_UNESCAPED_SLASHES); + $visible = substr($quoted, 1, -1) . ($truncated ? '… [truncated]' : ''); + + return htmlspecialchars($visible, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); + } +} diff --git a/.github/OptionalAnalysis/WorkflowInput.php b/.github/OptionalAnalysis/WorkflowInput.php new file mode 100644 index 0000000..5f633c9 --- /dev/null +++ b/.github/OptionalAnalysis/WorkflowInput.php @@ -0,0 +1,69 @@ + $environment Literal environment fields supplied by the composite action. */ + public static function fromEnvironment(array $environment): self + { + return new self( + self::field($environment, 'STANDARDS_DIRECTORY'), + self::field($environment, 'STANDARDS_BASE'), + self::field($environment, 'STANDARDS_HEAD'), + self::boolean(self::field($environment, 'STANDARDS_MAGIC_NUMBERS')), + self::field($environment, 'STANDARDS_MAGIC_NUMBERS_POLICY'), + self::boolean(self::field($environment, 'STANDARDS_RECTOR')), + self::field($environment, 'STANDARDS_RECTOR_POLICY'), + self::field($environment, 'STANDARDS_RECTOR_CONFIG'), + ); + } + + /** Missing fields fail explicitly rather than becoming a silently disabled analysis. */ + private static function field(array $environment, string $name): string + { + if (!isset($environment[$name]) || !is_string($environment[$name])) { + throw new InvalidArgumentException('Missing optional-analysis input: ' . $name); + } + + return $environment[$name]; + } + + /** GitHub booleans have a literal true/false representation; other strings are not coerced. */ + private static function boolean(string $value): bool + { + if (!in_array($value, ['true', 'false'], true)) { + throw new InvalidArgumentException('Optional analysis enable inputs must be true or false.'); + } + + return 'true' === $value; + } +} diff --git a/.github/OptionalAnalysis/WorkflowServiceProvider.php b/.github/OptionalAnalysis/WorkflowServiceProvider.php new file mode 100644 index 0000000..e725761 --- /dev/null +++ b/.github/OptionalAnalysis/WorkflowServiceProvider.php @@ -0,0 +1,38 @@ + The scalar checkout identity cannot be inferred by autowiring. */ + public function getFactories(): array + { + return [OptionalAnalysisRunner::class => new InvokableFactory( + OptionalAnalysisRunner::class, + FileSelection::class, + PhpFiles::class, + ProjectScopeResolver::class, + BoundedProcess::class, + NativeReportParser::class, + $this->root, + )]; + } + + /** @return array This helper decorates no public runtime service. */ + public function getExtensions(): array + { + return []; + } +} diff --git a/.github/OptionalAnalysis/launch.php b/.github/OptionalAnalysis/launch.php new file mode 100644 index 0000000..dde3693 --- /dev/null +++ b/.github/OptionalAnalysis/launch.php @@ -0,0 +1,23 @@ + ['file', '/dev/null', 'r'], 1 => STDOUT, 2 => STDERR], $pipes); +if (!is_resource($process)) { + exit(1); +} +exit(proc_close($process)); diff --git a/.github/actions/analyze/action.yml b/.github/actions/analyze/action.yml new file mode 100644 index 0000000..f9880dc --- /dev/null +++ b/.github/actions/analyze/action.yml @@ -0,0 +1,51 @@ +name: Analyze optional PHP quality +description: Classify explicitly enabled native analyzers and write a bounded job summary. +inputs: + directory: + required: true + description: Absolute exact consumer checkout directory. + base-sha: + required: true + description: Exact PR base commit. + head-sha: + required: true + description: Exact PR head commit. + magic-numbers: + default: 'false' + description: Explicitly enable native Magic Numbers analysis. + magic-numbers-policy: + default: report + description: report or fail for verified Magic Numbers findings; operational errors always block. + rector: + default: 'false' + description: Explicitly enable native Rector preview, never semantic apply. + rector-policy: + default: report + description: report or fail for verified Rector changes; operational errors always block. + rector-config: + default: '' + description: Required consumer-relative native PHP configuration when Rector is enabled. +outputs: + summary-written: + description: True only after the classified results were written to the job summary. + value: ${{ steps.report.outputs.summary-written }} + blocking: + description: Explicit policy decision; the workflow must enforce it in its final gate. + value: ${{ steps.report.outputs.blocking }} +runs: + using: composite + steps: + - name: Classify optional results and write summary + id: report + shell: bash + env: + STANDARDS_ROOT: ${{ github.action_path }}/../../.. + STANDARDS_DIRECTORY: ${{ inputs.directory }} + STANDARDS_BASE: ${{ inputs.base-sha }} + STANDARDS_HEAD: ${{ inputs.head-sha }} + STANDARDS_MAGIC_NUMBERS: ${{ inputs.magic-numbers }} + STANDARDS_MAGIC_NUMBERS_POLICY: ${{ inputs.magic-numbers-policy }} + STANDARDS_RECTOR: ${{ inputs.rector }} + STANDARDS_RECTOR_POLICY: ${{ inputs.rector-policy }} + STANDARDS_RECTOR_CONFIG: ${{ inputs.rector-config }} + run: php "$STANDARDS_ROOT/.github/run-optional-analysis.php" diff --git a/.github/run-optional-analysis.php b/.github/run-optional-analysis.php new file mode 100644 index 0000000..de2bf68 --- /dev/null +++ b/.github/run-optional-analysis.php @@ -0,0 +1,37 @@ +get(OptionalAnalysisRunner::class)->run($input); +$summary = $container->get(SummaryRenderer::class)->render($results); +$path = getenv('GITHUB_STEP_SUMMARY'); +if (!is_string($path) || '' === $path || strlen($summary) !== file_put_contents($path, $summary, FILE_APPEND)) { + throw new RuntimeException('Cannot write the optional-analysis summary.'); +} +$policies = ['Magic Numbers' => $input->magicNumbersPolicy, 'Rector' => $input->rectorPolicy]; +$failure = false; +foreach ($results as $result) { + $failure = $result->blocks($policies[$result->tool]) || $failure; +} +$output = getenv('GITHUB_OUTPUT'); +$values = "summary-written=true\nblocking=" . ($failure ? 'true' : 'false') . "\n"; +if (!is_string($output) || '' === $output || strlen($values) !== file_put_contents($output, $values, FILE_APPEND)) { + throw new RuntimeException('Cannot record optional-analysis summary completion and policy.'); +} +// Classification completed; the workflow's mandatory final gate applies this explicit blocking decision. +exit(0); diff --git a/.github/workflows/code-standards-analysis.yml b/.github/workflows/code-standards-analysis.yml new file mode 100644 index 0000000..1f56e6c --- /dev/null +++ b/.github/workflows/code-standards-analysis.yml @@ -0,0 +1,148 @@ +name: 'Internal: Optional PHP Analysis' +on: + workflow_call: + inputs: + php-version: + default: '8.3' + type: string + magic-numbers: + default: false + type: boolean + magic-numbers-policy: + default: report + type: string + rector: + default: false + type: boolean + rector-policy: + default: report + type: string + rector-config: + default: '' + type: string +permissions: + contents: read +jobs: + analyze: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Reject privileged events and invalid refs + id: runtime + env: + STANDARDS_EVENT: ${{ github.event_name }} + STANDARDS_JOB_JSON: ${{ toJSON(job) }} + STANDARDS_BASE: ${{ github.event.pull_request.base.sha }} + STANDARDS_HEAD: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + [[ "$STANDARDS_EVENT" == pull_request ]] || exit 1 + [[ "$STANDARDS_BASE" =~ ^[a-fA-F0-9]{40}$ ]] || exit 1 + [[ "$STANDARDS_HEAD" =~ ^[a-fA-F0-9]{40}$ ]] || exit 1 + runtime=$(jq --slurp --raw-output --exit-status ' + select(length == 1) | .[0] | select(type == "object") + | select(.workflow_repository | type == "string") + | select(.workflow_repository == "php-fast-forward/code-standards") + | select(.workflow_sha | type == "string") + | select(.workflow_sha | length == 40 and test("^[a-f0-9]+$")) + | .workflow_sha + ' <<< "$STANDARDS_JOB_JSON") || { + echo '::error::Invalid executing workflow identity in native job context.' >&2 + exit 1 + } + { + printf 'repository=php-fast-forward/code-standards\n' + printf 'sha=%s\n' "$runtime" + } >> "$GITHUB_OUTPUT" + - name: Validate explicit optional findings policies + env: + STANDARDS_MAGIC_POLICY: ${{ inputs.magic-numbers-policy }} + STANDARDS_RECTOR_POLICY: ${{ inputs.rector-policy }} + run: | + set -euo pipefail + for policy in "$STANDARDS_MAGIC_POLICY" "$STANDARDS_RECTOR_POLICY"; do + [[ "$policy" == report || "$policy" == fail ]] || { + echo '::error::Optional findings policies must be report or fail.' >&2 + exit 1 + } + done + - name: Check out reviewed tool runtime + if: inputs.magic-numbers || inputs.rector + uses: actions/checkout@v7 + with: + repository: ${{ steps.runtime.outputs.repository }} + ref: ${{ steps.runtime.outputs.sha }} + path: tool + persist-credentials: false + - name: Set up PHP + if: inputs.magic-numbers || inputs.rector + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ inputs.php-version }} + extensions: dom, posix + coverage: none + tools: composer:v2 + - name: Install only the reviewed tool + if: inputs.magic-numbers || inputs.rector + working-directory: tool + env: + COMPOSER_HOME: ${{ runner.temp }}/standards-analysis-composer-home + COMPOSER_CACHE_DIR: ${{ runner.temp }}/standards-analysis-composer-cache + run: composer install --no-dev --no-plugins --no-scripts --no-interaction --prefer-dist --no-progress + - name: Check out exact consumer head + if: inputs.magic-numbers || inputs.rector + uses: actions/checkout@v7 + with: + ref: ${{ github.event.pull_request.head.sha }} + path: consumer + fetch-depth: 0 + persist-credentials: false + - name: Classify optional analyses + id: analysis + if: inputs.magic-numbers || inputs.rector + uses: ./tool/.github/actions/analyze + with: + directory: ${{ github.workspace }}/consumer + base-sha: ${{ github.event.pull_request.base.sha }} + head-sha: ${{ github.event.pull_request.head.sha }} + magic-numbers: ${{ inputs.magic-numbers }} + magic-numbers-policy: ${{ inputs.magic-numbers-policy }} + rector: ${{ inputs.rector }} + rector-policy: ${{ inputs.rector-policy }} + rector-config: ${{ inputs.rector-config }} + - name: Always summarize unavailable or disabled analyses + if: always() && steps.analysis.outputs.summary-written != 'true' + env: + STANDARDS_MAGIC_ENABLED: ${{ inputs.magic-numbers }} + STANDARDS_RECTOR_ENABLED: ${{ inputs.rector }} + run: | + set -euo pipefail + { + printf '## Optional Code Standards analyses\n\n' + printf '| Tool | Status | Findings |\n| --- | --- | --- |\n' + if [[ "$STANDARDS_MAGIC_ENABLED" == true ]]; then + printf '| Magic Numbers | operational-error | unavailable |\n' + else + printf '| Magic Numbers | disabled | not executed |\n' + fi + if [[ "$STANDARDS_RECTOR_ENABLED" == true ]]; then + printf '| Rector | operational-error | unavailable |\n' + else + printf '| Rector | disabled | not executed |\n' + fi + printf '\nEnabled analyses without a complete classified result are operational errors, never clean passes.\n' + } >> "$GITHUB_STEP_SUMMARY" + - name: Enforce explicit optional-analysis result + if: always() + env: + STANDARDS_MAGIC_ENABLED: ${{ inputs.magic-numbers }} + STANDARDS_RECTOR_ENABLED: ${{ inputs.rector }} + STANDARDS_SUMMARY_WRITTEN: ${{ steps.analysis.outputs.summary-written }} + STANDARDS_BLOCKING: ${{ steps.analysis.outputs.blocking }} + run: | + set -euo pipefail + if [[ "$STANDARDS_MAGIC_ENABLED" != true && "$STANDARDS_RECTOR_ENABLED" != true ]]; then + exit 0 + fi + [[ "$STANDARDS_SUMMARY_WRITTEN" == true ]] || exit 1 + [[ "$STANDARDS_BLOCKING" == false ]] diff --git a/.github/workflows/code-standards.yml b/.github/workflows/code-standards.yml index 0f2fd20..64a300c 100644 --- a/.github/workflows/code-standards.yml +++ b/.github/workflows/code-standards.yml @@ -10,6 +10,26 @@ on: description: Optional consumer-relative ECS config. default: '' type: string + magic-numbers: + description: Explicitly enable native Magic Numbers analysis of the exact PR selection. + default: false + type: boolean + magic-numbers-policy: + description: report or fail for verified findings; operational errors always fail. + default: report + type: string + rector: + description: Explicitly enable native semantic preview using rector-config, never apply. + default: false + type: boolean + rector-policy: + description: report or fail for verified proposed changes; operational errors always fail. + default: report + type: string + rector-config: + description: Consumer-relative native PHP configuration with explicit target and rules. + default: '' + type: string permissions: {} jobs: check: @@ -19,3 +39,14 @@ jobs: with: php-version: ${{ inputs.php-version }} config: ${{ inputs.config }} + optional-analysis: + permissions: + contents: read + uses: ./.github/workflows/code-standards-analysis.yml + with: + php-version: ${{ inputs.php-version }} + magic-numbers: ${{ inputs.magic-numbers }} + magic-numbers-policy: ${{ inputs.magic-numbers-policy }} + rector: ${{ inputs.rector }} + rector-policy: ${{ inputs.rector-policy }} + rector-config: ${{ inputs.rector-config }} diff --git a/.github/workflows/repository-ci.yml b/.github/workflows/repository-ci.yml index 849c88b..8ca0c76 100644 --- a/.github/workflows/repository-ci.yml +++ b/.github/workflows/repository-ci.yml @@ -7,9 +7,6 @@ on: permissions: contents: read jobs: - reusable-check: - if: github.event_name == 'pull_request' - uses: ./.github/workflows/code-standards.yml tests: runs-on: ubuntu-latest timeout-minutes: 15 @@ -24,6 +21,7 @@ jobs: - uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} + extensions: dom, posix coverage: none tools: composer:v2 - run: composer install --no-plugins --no-scripts --no-interaction --prefer-dist --no-progress diff --git a/.github/workflows/repository-pr-standards.yml b/.github/workflows/repository-pr-standards.yml new file mode 100644 index 0000000..4d48bec --- /dev/null +++ b/.github/workflows/repository-pr-standards.yml @@ -0,0 +1,11 @@ +name: 'Repository: PR Standards' +on: + pull_request: +permissions: + contents: read +jobs: + reusable-check: + uses: ./.github/workflows/code-standards.yml + with: + magic-numbers: true + magic-numbers-policy: report diff --git a/README.md b/README.md index f9dbc27..60525cc 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ type analysis, application tests and coverage belong to their respective tools. - Read-only syntax/style checking and explicitly requested local fixes. - Exact PR merge-base selection with binary-safe file paths. - Invocable Symfony Console commands composed with Fast Forward Container. -- An unprivileged reusable workflow and a portable consumption skill. +- An unprivileged reusable workflow with explicit optional analysis summaries and a portable consumption skill. ## 📦 Installation and requirements @@ -163,12 +163,23 @@ static analysis remain separate; no DevTools extraction parity is asserted. ## 🧰 API summary +The reusable keeps optional analyses disabled by default. Enable +`magic-numbers: true` or `rector: true` explicitly; Rector also requires +`rector-config`. Each tool's `*-policy` accepts `report` or `fail` for verified +findings. Operational failures always fail, including invalid/incomplete native +reports. A visible summary distinguishes disabled, not executed, clean, findings +and operational error, with native counts and available file/line information. +See [optional workflow analyses](docs/optional-analysis.rst) for inputs, limits, +trust boundaries and consumer examples. The selected workflow revision defines +its input contract independently of Composer package constraints. + | Entry point | Contract | | --- | --- | | `code-standards check` | Parse selected PHP and check style without changing source. | | `code-standards fix` | Explicitly format selected source, then verify; failures may leave partial changes. | | `code-standards refactor --config=rector.php` | Preview native semantic rules; only `--apply` permits writes, followed by verification. | | [`code-standards magic-numbers`](docs/magic-numbers.rst) | Analyze selected source with opt-in PHPMND, independently of check/fix; findings keep a nonzero status. | +| `code-standards refactor --config=rector.php --format=json` | Emit native JSON for preview only; JSON cannot be combined with `--apply`. | | `ECSConfig::configure(?callable $customize)` | Return the native ECS builder; apply the optional callback last. | | `help check` / `help fix` / `help refactor` / `help magic-numbers` | Show the installed Symfony option definitions. | diff --git a/docs/automation.rst b/docs/automation.rst index e4f9689..85fdf28 100644 --- a/docs/automation.rst +++ b/docs/automation.rst @@ -14,6 +14,8 @@ Following Changelog's separation: +-------------------------------------------------------+--------------------------------------------------------------------------------------------+ | ``.github/workflows/repository-ci.yml`` | This project's own tests, style, workflow contracts | +-------------------------------------------------------+--------------------------------------------------------------------------------------------+ +| ``.github/workflows/repository-pr-standards.yml`` | This project's PR-only consumer call, separate from push and manual project tests | ++-------------------------------------------------------+--------------------------------------------------------------------------------------------+ | ``.github/workflows/repository-changelog.yml`` | Chosen Changelog lifecycle for this project's approved 0.x releases | +-------------------------------------------------------+--------------------------------------------------------------------------------------------+ | ``.github/workflows/repository-release-channels.yml`` | Internal released-only v0 update after successful publication; explicit recovery from main | @@ -23,6 +25,8 @@ Following Changelog's separation: | ``resources/skills/fast-forward-code-standards`` | Canonical portable consumer skill | +-------------------------------------------------------+--------------------------------------------------------------------------------------------+ +The project's own consumer call is declared in ``repository-pr-standards.yml`` with only the native ``pull_request`` trigger. Its ``reusable-check`` job retains the same inputs and nested job IDs; the top-level workflow is named ``Repository: PR Standards``. ``Repository: Quality`` retains its PHP matrix and project checks on PRs, main pushes and manual dispatch. This separates the PR-only execution surface from events that can write default-branch caches without changing required-check settings. + The reusable runs only on ``pull_request``, on an ephemeral ``ubuntu-latest`` runner, with ``contents: read``. It rejects privileged target events before checkout. It uses separate sibling tool and consumer checkouts, exact consumer head SHA, full Git history and ``persist-credentials: false``. No application dependencies, plugins or scripts are installed. Only the tool's locked dependencies are installed with plugins/scripts disabled. Config PHP executes without supplied secrets; consumers must never call this check from a privileged workflow. The selected consumer interface uses ``@v0`` after a published non-prerelease 0.x release has established that GitHub branch. A reviewed full commit remains available for fixed adoption. The reusable exports the native ``job`` context through the supported ``toJSON(job)`` expression in step environment variables. Before checkout, an inline jq guard validates exactly one JSON object, a string repository equal to ``php-fast-forward/code-standards``, and a string SHA containing exactly 40 lowercase hexadecimal characters. Only that validated identity is emitted as step outputs and used for checkout. Invalid, absent or multiple documents emit no checkout outputs and fail closed. The workflow never resolves the moving branch again. ``github.workflow_sha`` identifies the caller and is not used for this purpose; no caller-supplied runtime ref can silently drift from the workflow. The same protocol protects the channel publisher's trusted implementation. The four native workflow identity properties are GitHub.com features; a host without them, including GitHub Enterprise Server, fails this protocol. diff --git a/docs/index.rst b/docs/index.rst index 03015bf..43416ba 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -27,6 +27,7 @@ Architecture, policy and evidence architecture automation instruction-map + optional-analysis legacy-fixer-comparison magic-numbers phpdoc-comparison diff --git a/docs/optional-analysis.rst b/docs/optional-analysis.rst new file mode 100644 index 0000000..d09493d --- /dev/null +++ b/docs/optional-analysis.rst @@ -0,0 +1,144 @@ +Optional workflow analyses and summaries +======================================== + +The public reusable keeps the mandatory style/syntax check unchanged. A separate +hosted job reports optional analysis without making ``check`` or ``fix`` invoke +semantic or quality tools. Both analyzers are **disabled by default**. With no +optional tool enabled, that job writes disabled statuses without checking out, +installing or executing another tool graph. + +Inputs and adoption +------------------- + +The existing ``php-version`` and ``config`` inputs retain their contracts. +``config`` belongs to the formatter; it does not enable or configure Rector. +The additional inputs are: + +* ``magic-numbers``: boolean, default ``false``; analyze the exact eligible PHP + selection through the separate ``magic-numbers`` command. +* ``magic-numbers-policy``: ``report`` (default) or ``fail`` for verified findings. +* ``rector``: boolean, default ``false``; run native semantic preview only. +* ``rector-config``: required when Rector is enabled; an existing consumer-relative + PHP configuration resolving inside the checkout, with explicit target/rules. + Native configuration owns Rector's whole-project paths; these are not restricted + to the formatter changed-file list. No automatic ``--apply`` is possible. +* ``rector-policy``: ``report`` (default) or ``fail`` for verified proposed changes. + +Policies are exact strings; unsupported values fail explicitly. A policy only +changes the treatment of **verified findings**. Operational errors always block, +including when the selected policy is ``report``. Tools are not enabled because +their dependency is installed or a configuration filename happens to exist. + +The consumer wrapper in ``resources/workflows/code-standards.yml`` provides the +basic PR style check through the published ``v0`` channel. The fragment below +describes the optional-analysis inputs. The selected workflow revision defines +its supported input contract; callers supply inputs belonging to that revision. +GitHub workflow channels and Composer/CPX package constraints are separate +distribution choices. The unprivileged PR event and runtime/ref provenance +contract apply to both basic checks and explicitly enabled analyses. + +.. code-block:: yaml + + with: + php-version: '8.3' + magic-numbers: true + magic-numbers-policy: report + rector: true + rector-config: rector.php + rector-policy: fail + +Result states and counts +------------------------ + +The job summary distinguishes: + +* ``disabled``: not requested; no optional analyzer execution occurred. +* ``not-executed``: valid Magic Numbers selection contains no eligible PHP files; + no engine execution or clean native result is claimed. +* ``clean``: an executed, complete native report and exit agree on zero findings. +* ``findings``: complete native findings/differences; report-only policy informs, + while fail policy blocks. +* ``operational-error``: failed prerequisites, execution/timeout, malformed or + inconsistent output, missing capability or exceeded limits. Missing results + are not silently replaced with zero counts. + +For Magic Numbers, ``selected PHP files`` comes from the existing +``FileSelection``/``PhpFiles`` contract, not XML ``fileCount``. Native ``fileCount`` +counts only files containing findings. ``Findings`` counts native entries; their +real file/line values are shown. XML columns are not safe AST-patch identities. +An empty eligible selection is checked before launching the analyzer. +Selection runs only when Magic Numbers is enabled; its failure does not prevent +Rector from producing its own result using the configured whole-project paths. + +For Rector, ``changed files`` is the native total, not a count of individual +rules, lines or defects. Available native error locations retain their line +field. Proposed changes have no native JSON line field; the summary marks that +location unavailable rather than parsing a unified diff or inventing precision. +The explicit CLI JSON representation retains the native diff for reviewed use. + +Classification requires the complete structured report **and** the process exit. +PHPMND exit 1 alone can mean findings or failure. Rector exit 2 alone can mean +proposed changes or a controlled invocation error. Unsupported combinations fail +closed. A mixed Rector report containing errors remains operational failure even +when it also includes proposed changes. + +Execution, summary and trust limits +----------------------------------- + +The workflow uses only ``pull_request``, ``contents: read``, GitHub-hosted Ubuntu, +the called job's verified repository/SHA, exact consumer head, separate checkouts +and no persisted checkout credentials. Only the tool's lock is installed, with +plugins/scripts disabled; consumer dependencies are not installed. Configured +optional tools do not change protections, required checks, releases or consumers. + +Each optional invocation has a total 300-second budget, a 2 MiB stdout report cap +and a separate 64 KiB diagnostic cap. The total Rector budget includes its native +inventory and processing, whereas the standalone CLI timeout is per subprocess. +The existing Magic Numbers argv bound is preserved. Overflow discards classification +of the incomplete report and reports operational error with unavailable counts; +it is not a truncated clean result. + +The workflow starts the Code Standards PHP CLI with ``memory_limit=256M``. +This constrains that PHP process, including its buffered native Rector preflight +output. It is not a global process-tree or native Rector memory limit: PHP +subprocesses have their own configuration, and executable configuration may alter +its own INI settings. No persistent INI file or standalone CLI default is changed. + +The workflow-private Linux executor starts a trusted launcher in its own +``setsid`` session, checks the child's PID/session/process-group identity and only +then permits the payload. It reads fixed nonblocking chunks with deadline checks; +it does not depend on a Symfony callback after unbounded intermediate pipe reads. +Only that confirmed owned group is terminated on completion/failure, including +ordinary analyzer descendants. Missing ``setsid`` or PHP POSIX support fails before +launch instead of choosing a weaker fallback. Invocation temporary files belong +to an owned temporary directory; cleanup does not follow symlinks. These are +workflow requirements, not new global Composer/CLI platform requirements. +The private child environment omits the existing tool credential keys plus +GitHub command-file paths and known Actions token fields. CI/workspace metadata +remain available. This narrows ambient capabilities; it does not claim that +arbitrary PHP is sandboxed from the runner filesystem. + +Reports are parsed without network/external-entity resolution and XML DTDs are +rejected. Root/files/file structural boundaries allow only the expected elements, +whitespace text and comments; other nodes, including CDATA and processing +instructions, reject classification. CDATA inside native source snippets remains +payload. The summary displays bounded, escaped preformatted text; source snippets, +full diffs and untrusted HTML/Markdown are not interpreted. Control/non-ASCII +characters use visible JSON escapes. Display is limited to 100 diagnostics per +tool and 64 KiB overall with explicit omission/truncation notices; validated +native totals are not reduced to the displayed subset. Raw reports are not +uploaded as artifacts or shared automatically. + +The classification step writes its summary and a controlled blocking decision. +A mandatory final gate fails for operational errors, configured blocking findings, +or incomplete/missing outputs; there is no blanket ``continue-on-error``. +An ``always()`` fallback writes unavailable/error statuses if setup or the helper +fails before it can provide a complete result. Platform cancellation or outages +can prevent cleanup/final steps and never establish a successful analysis. + +Rector configuration, custom rules and bootstraps are executable PHP and can +mutate files even during preview. This is not a security/filesystem sandbox or +a guarantee against malicious code which detaches another session. The boundary +is the unprivileged ephemeral hosted job and finite runtime. Never call it from +a privileged target event or supply secrets. Review the +:doc:`native refactoring contract ` before enabling it. diff --git a/docs/rector.rst b/docs/rector.rst index 9bca61b..947873d 100644 --- a/docs/rector.rst +++ b/docs/rector.rst @@ -34,6 +34,35 @@ Preview passes native ``--dry-run``. Explicit apply performs one write pass and ``--timeout`` accepts a positive whole number of seconds and defaults to 300. It bounds each Rector subprocess separately: preflight inventory, processing and post-apply verification receive the same limit. Fractional, negative, zero, nonfinite and overflowing values are rejected; no unlimited mode is exposed. The option changes this refactor invocation, leaving Git's 120-second captured transport default and formatting's 300-second default unchanged. Native Rector process/parallel-worker limits remain native configuration decisions. +Native JSON preview +------------------- + +``--format`` accepts ``text`` (the default) or ``json``. JSON selects Rector's +native output formatter for preview, preserving its totals, errors, +``changed_files`` and ``file_diffs`` without scraping or rewriting them. +JSON and ``--apply`` are rejected together before evaluating configuration or +starting native work: apply plus verification would otherwise concatenate two +documents. Text apply and its verification keep the existing behavior. + +.. code-block:: console + + php bin/code-standards refactor --config=rector.php --format=json > rector-preview.json + +The redirection writes only the explicitly selected report file. Native JSON +has ``totals.changed_files`` and ``totals.errors``; available processing errors +carry file/message/line fields. Proposed changes carry files and native diffs, +not an AST-location field. The reusable does not infer source lines from diff +text. Clean preview returns 0; native changes return 2; processing errors +return 1. Controlled invocation errors can also return 2, so automation must +validate the complete report together with its exit status. An unavailable or +malformed report is an operational error, never a clean or advisory finding. + +This representation adds no inventory or processing invocation. Executable +configuration and its stdout can still affect execution/report validity; JSON +preview retains the trust and namespace boundaries below. The +`optional workflow guide `__ describes explicitly enabled +Rector preview, report/fail policies and bounded summaries. + Executable configuration and initialization boundary ---------------------------------------------------- diff --git a/docs/usage/commands.rst b/docs/usage/commands.rst index 1a23ec3..20b64aa 100644 --- a/docs/usage/commands.rst +++ b/docs/usage/commands.rst @@ -12,6 +12,9 @@ configuration's source paths rather than this Git changed-file selection. See the `semantic refactoring contract <../rector.rst>`__ for required target PHP, optional native cache configuration, bounded timeouts and execution limits. Use ``help refactor`` for its installed options. +Native JSON preview is explicit with ``--format=json``; it cannot be combined +with ``--apply``. Human text remains the default. See the +`optional workflow analyses <../optional-analysis.rst>`__ for CI policies and summaries. The opt-in ``magic-numbers`` command uses the same Git selection as ``check`` and ``fix``, independently of formatting. It accepts ``--all`` or ``--base``/``--head`` diff --git a/resources/workflows/code-standards.yml b/resources/workflows/code-standards.yml index 8e9e31b..eee55f9 100644 --- a/resources/workflows/code-standards.yml +++ b/resources/workflows/code-standards.yml @@ -1,4 +1,3 @@ -# Copy after the first real 0.x release has established the published v0 channel. name: PHP standards on: pull_request: diff --git a/src/Console/Command/RefactorCommand.php b/src/Console/Command/RefactorCommand.php index 439be77..5ec154e 100644 --- a/src/Console/Command/RefactorCommand.php +++ b/src/Console/Command/RefactorCommand.php @@ -40,6 +40,9 @@ public function __invoke( #[Option(description: 'Positive whole seconds per Rector subprocess, including preflight and verification.')] string $timeout = '300', + + #[Option(description: 'Native output: text, or json for preview only; json cannot be combined with --apply.')] + string $format = 'text', ): int { try { if (null === $config) { @@ -49,7 +52,7 @@ public function __invoke( $seconds = $this->timeoutSeconds($timeout); $scope = $this->projects->resolve(true, null, null, $directory, $config); - return $this->rector->run($scope->directory, $scope->config, $apply, $seconds); + return $this->rector->run($scope->directory, $scope->config, $apply, $seconds, $format); } catch (ProcessFailedException $error) { $io->getErrorStyle()->error($error->getMessage()); diff --git a/src/Refactoring/RectorRunner.php b/src/Refactoring/RectorRunner.php index adbc67d..6790d45 100644 --- a/src/Refactoring/RectorRunner.php +++ b/src/Refactoring/RectorRunner.php @@ -22,12 +22,25 @@ public function __construct(private ProcessRunner $process, private string $rect * successful apply adds verification. Native Rector may evaluate configuration more than once per invocation. * This transport is not a filesystem sandbox. * + * JSON is native preview output only; apply plus verification would produce multiple documents. + * Invalid output/write combinations fail before evaluating executable configuration. + * * @return int Native process status, including 2 when dry-run proposes changes. */ - public function run(string $directory, string $config, bool $apply, float $timeout = 300): int + public function run(string $directory, string $config, bool $apply, float $timeout = 300, string $format = 'text'): int { + if (!in_array($format, ['text', 'json'], true)) { + throw new InvalidArgumentException('Rector format must be text or json.'); + } + if ($apply && 'json' === $format) { + throw new InvalidArgumentException('JSON output is preview-only; use text output for explicit --apply.'); + } + $this->assertRules($directory, $config, $timeout); $check = [PHP_BINARY, $this->rector, 'process', '--config=' . $config, '--no-progress-bar']; + if ('json' === $format) { + $check[] = '--output-format=json'; + } $status = $this->process->run($apply ? $check : [...$check, '--dry-run'], $directory, $timeout); if (!$apply || 0 !== $status) { return $status; diff --git a/tests/Automation/OptionalAnalysisEntrypointTest.php b/tests/Automation/OptionalAnalysisEntrypointTest.php new file mode 100644 index 0000000..63a631e --- /dev/null +++ b/tests/Automation/OptionalAnalysisEntrypointTest.php @@ -0,0 +1,88 @@ +directory = realpath(sys_get_temp_dir()) . '/optional-entrypoint-' . bin2hex(random_bytes(8)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->directory); + } + + /** Disabled status requires neither an existing consumer directory nor Git/native execution. */ + public function testCompositionWritesBothDisabledStatesAndACompleteControlledDecision(): void + { + $process = $this->process(dirname(__DIR__, 2) . '/.github/run-optional-analysis.php'); + self::assertSame(0, $process->run(), $process->getErrorOutput()); + self::assertSame("summary-written=true\nblocking=false\n", file_get_contents($this->directory . '/outputs')); + $summary = file_get_contents($this->directory . '/summary'); + self::assertStringContainsString('| Magic Numbers | disabled |', $summary); + self::assertStringContainsString('| Rector | disabled |', $summary); + self::assertStringNotContainsString('| clean |', $summary); + } + + /** @return iterable Each runner command file must be written completely. */ + public static function partialTargets(): iterable + { + yield 'summary' => ['GITHUB_STEP_SUMMARY']; + yield 'outputs' => ['GITHUB_OUTPUT']; + } + + /** A partial write is not a successfully summarized/classified invocation. */ + #[DataProvider('partialTargets')] + public function testPartialRunnerFileWritesFailInsteadOfClaimingCompletion(string $target): void + { + $source = <<<'PHP' + remaining, strlen($data)); + $this->remaining -= $written; + return $written; + } + public function stream_flush(): bool { return true; } + } + stream_wrapper_register('standards-partial', PartialRunnerFile::class); + PHP; + $bootstrap = $this->directory . '/partial.php'; + file_put_contents($bootstrap, $source . "\nrequire " . var_export(dirname(__DIR__, 2) . '/.github/run-optional-analysis.php', true) . ';'); + $process = $this->process($bootstrap, [$target => 'standards-partial://runner-file']); + self::assertNotSame(0, $process->run()); + self::assertStringContainsString('Cannot ' . ('GITHUB_OUTPUT' === $target ? 'record' : 'write'), $process->getErrorOutput()); + if ('GITHUB_STEP_SUMMARY' === $target) { + self::assertFileDoesNotExist($this->directory . '/outputs'); + } + } + + /** Literal fixture inputs disable both analyzers; the absent consumer cannot be used accidentally. */ + private function process(string $entrypoint, array $overrides = []): Process + { + return new Process([PHP_BINARY, $entrypoint], dirname(__DIR__, 2), [ + 'STANDARDS_DIRECTORY' => '/nonexistent-optional-consumer', + 'STANDARDS_BASE' => str_repeat('a', 40), 'STANDARDS_HEAD' => str_repeat('b', 40), + 'STANDARDS_MAGIC_NUMBERS' => 'false', 'STANDARDS_MAGIC_NUMBERS_POLICY' => 'report', + 'STANDARDS_RECTOR' => 'false', 'STANDARDS_RECTOR_POLICY' => 'report', 'STANDARDS_RECTOR_CONFIG' => '', + 'GITHUB_STEP_SUMMARY' => $this->directory . '/summary', 'GITHUB_OUTPUT' => $this->directory . '/outputs', + ...$overrides, + ], timeout: 10); + } +} diff --git a/tests/Automation/OptionalAnalysisInputTest.php b/tests/Automation/OptionalAnalysisInputTest.php new file mode 100644 index 0000000..69697be --- /dev/null +++ b/tests/Automation/OptionalAnalysisInputTest.php @@ -0,0 +1,131 @@ +createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $process->expects(self::never())->method('run'); + $runner = $this->runner($process); + $results = $runner->run($this->input('/directory-that-does-not-exist', false)); + self::assertSame(['disabled', 'disabled'], array_column($results, 'status')); + } + + /** A valid Git selection with no PHP produces not-executed, never a fabricated engine-clean result. */ + public function testEmptySelectionDoesNotStartTheAnalyzer(): void + { + $directory = realpath(sys_get_temp_dir()); + $process = $this->createMock(ProcessRunner::class); + $process->method('capture')->willReturnCallback(static function (array $arguments): string { + return match (true) { + in_array('--is-inside-work-tree', $arguments, true) => "true\n", + in_array('rev-parse', $arguments, true) && in_array('HEAD', $arguments, true) => str_repeat('b', 40) . "\n", + default => '', + }; + }); + $process->expects(self::never())->method('run'); + $results = $this->runner($process)->run($this->input($directory, true)); + self::assertSame('not-executed', $results[0]->status); + self::assertSame(0, $results[0]->selectedCount); + self::assertNull($results[0]->findingCount); + self::assertNull($results[0]->nativeExit); + self::assertSame('disabled', $results[1]->status); + } + + /** Unavailable checkout prerequisites are errors even when findings would be advisory. */ + public function testUnavailableConsumerCannotBecomeCleanOrAdvisoryFindings(): void + { + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $results = $this->runner($process)->run($this->input('/directory-that-does-not-exist', true)); + self::assertSame('operational-error', $results[0]->status); + self::assertTrue($results[0]->blocks('report')); + self::assertNull($results[0]->findingCount); + } + + /** Rector validates its native configuration without consulting Magic Numbers' changed-file scope. */ + public function testRectorOnlyDoesNotSelectMagicNumbersFiles(): void + { + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $process->expects(self::never())->method('run'); + $results = $this->runner($process)->run(new WorkflowInput( + realpath(sys_get_temp_dir()), str_repeat('a', 40), str_repeat('b', 40), false, 'report', true, 'report', '', + )); + self::assertSame('disabled', $results[0]->status); + self::assertSame('operational-error', $results[1]->status); + self::assertSame('Enabled Rector requires a consumer-relative rector-config input.', $results[1]->details); + } + + /** Selection errors belong to Magic Numbers and cannot replace Rector's independent result. */ + public function testMagicNumbersSelectionFailureDoesNotPreventRectorValidation(): void + { + $process = $this->createMock(ProcessRunner::class); + $process->method('capture')->willThrowException(new RuntimeException('Selection unavailable.')); + $process->expects(self::never())->method('run'); + $results = $this->runner($process)->run(new WorkflowInput( + realpath(sys_get_temp_dir()), str_repeat('a', 40), str_repeat('b', 40), true, 'report', true, 'report', '', + )); + self::assertSame('operational-error', $results[0]->status); + self::assertSame('Selection unavailable.', $results[0]->details); + self::assertNull($results[0]->selectedCount); + self::assertTrue($results[0]->blocks('report')); + self::assertSame('operational-error', $results[1]->status); + self::assertSame('Enabled Rector requires a consumer-relative rector-config input.', $results[1]->details); + } + + /** Missing/stringly enablement and unrecognized severity policies fail instead of being coerced. */ + public function testEnvironmentInputsAreExplicitAndStrict(): void + { + $environment = [ + 'STANDARDS_DIRECTORY' => '/consumer', 'STANDARDS_BASE' => str_repeat('a', 40), 'STANDARDS_HEAD' => str_repeat('b', 40), + 'STANDARDS_MAGIC_NUMBERS' => 'false', 'STANDARDS_MAGIC_NUMBERS_POLICY' => 'report', + 'STANDARDS_RECTOR' => 'false', 'STANDARDS_RECTOR_POLICY' => 'report', 'STANDARDS_RECTOR_CONFIG' => '', + ]; + self::assertFalse(WorkflowInput::fromEnvironment($environment)->magicNumbers); + foreach (['STANDARDS_MAGIC_NUMBERS' => 'yes', 'STANDARDS_MAGIC_NUMBERS_POLICY' => 'ignore', 'STANDARDS_HEAD' => 'main'] as $key => $value) { + try { + WorkflowInput::fromEnvironment([...$environment, $key => $value]); + self::fail('Malformed optional input must not be coerced.'); + } catch (InvalidArgumentException $exception) { + self::assertNotSame('', $exception->getMessage()); + } + } + } + + /** The fake installation is deliberately absent: tests which accidentally launch it must fail. */ + private function runner(ProcessRunner $process): OptionalAnalysisRunner + { + return new OptionalAnalysisRunner(new FileSelection($process), new PhpFiles(), new ProjectScopeResolver('/unused', new ProjectDirectoryResolver()), new BoundedProcess(), new NativeReportParser(), '/nonexistent-tool'); + } + + /** Produces only explicit fixture options; no native analysis executes inside this test class. */ + private function input(string $directory, bool $magic): WorkflowInput + { + return new WorkflowInput($directory, str_repeat('a', 40), str_repeat('b', 40), $magic, 'report', false, 'report', ''); + } +} diff --git a/tests/Automation/OptionalAnalysisProcessTest.php b/tests/Automation/OptionalAnalysisProcessTest.php new file mode 100644 index 0000000..53482c4 --- /dev/null +++ b/tests/Automation/OptionalAnalysisProcessTest.php @@ -0,0 +1,144 @@ +directory = realpath(sys_get_temp_dir()) . '/optional process fixture-' . bin2hex(random_bytes(8)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + if (isset($this->directory)) { + (new Filesystem())->remove($this->directory); + } + } + + /** Literal arguments, exact raw channels and nonzero status survive without inherited tool credentials. */ + public function testBoundedCaptureRetainsChannelsAndStatus(): void + { + $previous = []; + $fields = ['GH_TOKEN', 'GITHUB_TOKEN', 'COMPOSER_AUTH', 'GITHUB_OUTPUT', 'GITHUB_STEP_SUMMARY', 'GITHUB_ENV', 'GITHUB_PATH', 'GITHUB_STATE', 'ACTIONS_RUNTIME_TOKEN', 'ACTIONS_ID_TOKEN_REQUEST_TOKEN', 'ACTIONS_ID_TOKEN_REQUEST_URL']; + foreach ($fields as $name) { + $previous[$name] = getenv($name); + putenv($name . '=synthetic-test-value'); + } + $metadata = ['CI' => 'true', 'GITHUB_ACTIONS' => 'true', 'GITHUB_WORKSPACE' => $this->directory]; + foreach ($metadata as $name => $value) { + $previous[$name] = getenv($name); + putenv($name . '=' . $value); + } + try { + $source = 'foreach (' . var_export($fields, true) . ' as $name) { if (getenv($name)!==false) exit(19); } ' + . 'foreach (' . var_export($metadata, true) . ' as $name => $value) { if (getenv($name)!==$value) exit(20); } ' + . 'fwrite(STDOUT,$argv[1]); fwrite(STDERR,"error\0\xff"); exit(2);'; + $argument = "quote' space\nliteral"; + $result = (new BoundedProcess())->capture([PHP_BINARY, '-r', $source, $argument], $this->directory, 5); + self::assertSame(2, $result->exitCode); + self::assertSame($argument, $result->stdout); + self::assertSame("error\0\xff", $result->stderr); + } finally { + foreach ($previous as $name => $value) { + putenv(false === $value ? $name : $name . '=' . $value); + } + } + } + + /** A continuous producer cannot fill an unbounded intermediate buffer or leave its sleeping child alive. */ + public function testContinuousProducerAndOwnedDescendantStopAtTheCaptureLimit(): void + { + $pid = $this->directory . '/overflow-child.pid'; + try { + (new BoundedProcess())->capture([PHP_BINARY, '-r', $this->descendantSource('while (true) { fwrite(STDOUT, str_repeat("x", 8192)); }'), $pid], $this->directory, 5); + self::fail('Continuous output must fail at the capture cap.'); + } catch (RuntimeException $exception) { + self::assertStringContainsString('capture limit', $exception->getMessage()); + } + $this->assertOwnedChildTerminated($pid); + } + + /** A silent parent and sleeping child share the verified group and cannot evade the total timeout. */ + public function testTimeoutStopsOnlyTheOwnedProcessGroup(): void + { + $pid = $this->directory . '/timeout-child.pid'; + try { + (new BoundedProcess())->capture([PHP_BINARY, '-r', $this->descendantSource('sleep(60);'), $pid], $this->directory, 1); + self::fail('A silent invocation must have a finite total budget.'); + } catch (RuntimeException $exception) { + self::assertStringContainsString('timeout', $exception->getMessage()); + } + $this->assertOwnedChildTerminated($pid); + } + + /** stderr has its own small cap, independent of an otherwise valid stdout document. */ + public function testDiagnosticCaptureIsBoundedSeparately(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('capture limit'); + (new BoundedProcess())->capture([PHP_BINARY, '-r', 'fwrite(STDOUT,"{}"); while (true) { fwrite(STDERR,str_repeat("e",8192)); }'], $this->directory, 5); + } + + /** Cleanup unlinks owned path replacements without following a link to another fixture's contents. */ + public function testOwnedTemporaryCleanupNeverFollowsSymlinks(): void + { + $outside = $this->directory . '/preserved'; + mkdir($outside); + file_put_contents($outside . '/marker', 'preserve this owned fixture'); + foreach (['child', 'root'] as $placement) { + $source = '$own=getenv("TMPDIR"); if($argv[2]==="root"){rmdir($own);symlink($argv[1],$own);}else{symlink($argv[1],$own."/link");file_put_contents($own."/temporary","owned");} echo $own;'; + $result = (new BoundedProcess())->capture([PHP_BINARY, '-r', $source, $outside, $placement], $this->directory, 5); + self::assertSame(0, $result->exitCode); + self::assertFileDoesNotExist($result->stdout); + self::assertFalse(is_link($result->stdout)); + self::assertSame('preserve this owned fixture', file_get_contents($outside . '/marker')); + } + } + + /** The PID is written only by an exclusively owned synthetic sleeper, not discovered through a process listing. */ + private function descendantSource(string $parent): string + { + return '$child=proc_open([PHP_BINARY,"-r",\'file_put_contents($argv[1],(string)getmypid());sleep(60);\',$argv[1]],[0=>["file","/dev/null","r"],1=>["file","/dev/null","w"],2=>["file","/dev/null","w"]],$pipes);' + . '$deadline=microtime(true)+2;while(!is_file($argv[1])&µtime(true)<$deadline){usleep(1000);}if(!is_file($argv[1]))exit(20);' + . $parent; + } + + /** Reads only the known fixture PID's Linux state; a terminated zombie is not treated as a live orphan. */ + private function assertOwnedChildTerminated(string $file): void + { + self::assertFileExists($file); + $pid = file_get_contents($file); + self::assertMatchesRegularExpression('/\A[1-9][0-9]*\z/D', $pid); + $deadline = microtime(true) + 2; + do { + $state = @file_get_contents('/proc/' . $pid . '/stat'); + if (false === $state || in_array(substr($state, strrpos($state, ') ') + 2, 1), ['Z', 'X'], true)) { + self::addToAssertionCount(1); + + return; + } + usleep(10000); + } while (microtime(true) < $deadline); + + self::fail('The exclusively owned fixture descendant remains alive after cleanup.'); + } +} diff --git a/tests/Automation/OptionalAnalysisReportTest.php b/tests/Automation/OptionalAnalysisReportTest.php new file mode 100644 index 0000000..3957cc0 --- /dev/null +++ b/tests/Automation/OptionalAnalysisReportTest.php @@ -0,0 +1,232 @@ +magicNumbers(new ProcessResult(0, $this->xml(), ''), ['/consumer/a.php', '/consumer/b.php']); + self::assertSame('clean', $clean->status); + self::assertSame(2, $clean->selectedCount); + self::assertSame(0, $clean->findingCount); + + $found = $parser->magicNumbers(new ProcessResult(1, $this->xml('";]]>', 1, 1), ''), ['/consumer/a.php']); + self::assertSame('findings', $found->status); + self::assertSame(1, $found->findingCount); + self::assertSame(7, $found->diagnostics[0]['line']); + self::assertFalse($found->blocks('report')); + self::assertTrue($found->blocks('fail')); + } + + /** @return iterable Invalid/partial output must never be accepted as advisory debt. */ + public static function invalidMagicReports(): iterable + { + yield 'not XML' => ['not XML', 1]; + yield 'truncated XML' => ['', 1]; + yield 'wrong exit' => ['', 1]; + yield 'overflow' => ['', 0]; + yield 'counts' => ['', 1]; + yield 'unselected path' => ['', 1]; + yield 'entity declaration' => [']>', 0]; + } + + /** XML schema/status failures reject the report rather than returning a clean zero. */ + #[DataProvider('invalidMagicReports')] + public function testInvalidMagicReportIsOperationalFailure(string $report, int $status): void + { + $this->expectException(InvalidArgumentException::class); + (new NativeReportParser())->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + } + + /** @return iterable Unexpected nodes are rejected equally at all traversed structural boundaries. */ + public static function unexpectedStructuralNodes(): iterable + { + foreach ([ + 'text' => 'unexpected', + 'CDATA' => '', + 'whitespace CDATA' => '', + 'processing instruction' => '', + ] as $label => $node) { + yield 'root ' . $label => ['' . $node . '', 0]; + yield 'files ' . $label => ['' . $node . '', 0]; + yield 'file ' . $label => ['' . $node . '', 1]; + } + } + + /** Structural CDATA/PI must not be silently discarded before classifying a complete report. */ + #[DataProvider('unexpectedStructuralNodes')] + public function testUnexpectedStructuralNodesRejectClassification(string $report, int $status): void + { + $this->expectException(InvalidArgumentException::class); + (new NativeReportParser())->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + } + + /** Structural whitespace/comments are harmless; actual snippet CDATA remains native source payload. */ + public function testStructuralWhitespaceAndCommentsPreserveLegitimateSnippetCdata(): void + { + $report = '' . "\n\n" + . '' . "\n\n" + . '' . "\n\n" + . '";]]>' + . "\n\n\n"; + $result = (new NativeReportParser())->magicNumbers(new ProcessResult(1, $report, ''), ['/consumer/a.php']); + self::assertSame('findings', $result->status); + self::assertSame(1, $result->findingCount); + self::assertSame(7, $result->diagnostics[0]['line']); + } + + /** Rejecting a DTD must not first open its external subset or entity through a PHP stream wrapper. */ + public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void + { + if (!extension_loaded('dom')) { + self::markTestSkipped('External-entity qualification requires the real DOM extension.'); + } + self::assertTrue(stream_wrapper_register('standards-report-probe', ExternalEntityProbe::class)); + ExternalEntityProbe::$opens = 0; + try { + foreach ([ + '', + ']>', + ] as $declaration) { + $report = $declaration . '&source;'; + try { + (new NativeReportParser())->magicNumbers(new ProcessResult(0, $report, ''), []); + self::fail('DTD-bearing reports must be rejected.'); + } catch (InvalidArgumentException) { + self::assertSame(0, ExternalEntityProbe::$opens); + } + } + } finally { + self::assertTrue(stream_wrapper_unregister('standards-report-probe')); + } + } + + /** Native errors dominate report-only policy even when changes were also proposed. */ + public function testRectorNativeChangesAndErrorsHaveSeparateMeaning(): void + { + $parser = new NativeReportParser(); + $clean = $parser->rector(new ProcessResult(0, '{"totals":{"changed_files":0,"errors":0}}', '')); + self::assertSame('clean', $clean->status); + self::assertNull($clean->selectedCount); + + $json = '{"totals":{"changed_files":1,"errors":0},"changed_files":["src/a.php"],"file_diffs":[{"file":"src/a.php","diff":"@@ -1 +9 @@\\n-old\\n+new"}]}'; + $changes = $parser->rector(new ProcessResult(2, $json, '')); + self::assertSame('findings', $changes->status); + self::assertSame(1, $changes->findingCount); + self::assertNull($changes->diagnostics[0]['line'], 'Do not infer a line from a diff or fabricate AST identity.'); + self::assertFalse($changes->blocks('report')); + + $error = $parser->rector(new ProcessResult(1, '{"totals":{"changed_files":0,"errors":1},"errors":[{"file":"src/b.php","line":12,"message":"native failure"}]}', '')); + self::assertSame('operational-error', $error->status); + self::assertSame(12, $error->diagnostics[0]['line']); + self::assertTrue($error->blocks('report')); + self::assertTrue($error->blocks('fail')); + self::assertSame(1, $error->nativeExit); + } + + /** @return iterable ANSI, extra documents and mismatched types are not scraped away. */ + public static function invalidRectorReports(): iterable + { + yield 'human output' => ['Error: malformed config', 2]; + yield 'multiple JSON documents' => ['{"totals":{"changed_files":0,"errors":0}}{}', 0]; + yield 'coerced count' => ['{"totals":{"changed_files":"0","errors":0}}', 0]; + yield 'findings without list' => ['{"totals":{"changed_files":1,"errors":0}}', 2]; + yield 'clean with failure exit' => ['{"totals":{"changed_files":0,"errors":0}}', 1]; + yield 'errors without entries' => ['{"totals":{"changed_files":0,"errors":1}}', 1]; + } + + /** Controlled invocation errors sharing status 2 do not become Rector findings. */ + #[DataProvider('invalidRectorReports')] + public function testInvalidRectorReportIsOperationalFailure(string $report, int $status): void + { + $this->expectException(InvalidArgumentException::class); + (new NativeReportParser())->rector(new ProcessResult($status, $report, '')); + } + + /** Plain preformatted HTML keeps Markdown/commands inert and bounds display without changing native totals. */ + public function testSummaryEscapesAndTruncatesUntrustedDiagnostics(): void + { + $malicious = "\n[link](https://example.invalid) ::error::"; + $diagnostics = array_fill(0, 1000, ['file' => $malicious . str_repeat('x', 5000), 'line' => 7, 'message' => $malicious]); + $summary = (new SummaryRenderer())->render([ + new AnalysisResult('Magic Numbers', 'findings', 1, 1000, $diagnostics, $malicious, 1), + new AnalysisResult('Rector', 'disabled'), + ]); + self::assertLessThanOrEqual(65536, strlen($summary)); + self::assertStringNotContainsString('