Repository navigation
chore: sync with upstream 2.40.0 - #38
Conversation
|
📦 Target Release Version: |
| const dest = fs.mkdtempSync(src + '-verbose-'); | ||
| await fs.promises.cp(src, dest, {recursive: true, dereference: true}); | ||
| const scripts = path.join(dest, 'scripts'); | ||
| const verbose_run = path.join(scripts, path.basename(run_path)); |
🔍 Cherry-Pick Verification Report📦 Upstream Changes: 📋 File-by-File Analysis:
|
PR ReviewAction TypeNode-based action (runs on Passed Checks
Failed Checks
Warnings
Security Findings
SummaryThis PR syncs substantive upstream features (PHP 8.7 nightly support, tool checksum pinning, verbose debug mode, new tools cpx/laravel-cloud). The core implementation is solid and well-tested, but the README sync incorrectly reverts several step-security links to the upstream shivammathur org — specifically the CI badge, self-hosted runner wiki links, and a broken CONTRIBUTING.md reference — which must be fixed before merging. |
| <a href="https://github.com/step-security/setup-php" title="GitHub action to setup PHP"><img alt="GitHub Actions status" src="https://github.com/step-security/setup-php/workflows/Main%20workflow/badge.svg"></a> | ||
| <a href="https://github.com/step-security/setup-php/blob/main/LICENSE" title="license"><img alt="LICENSE" src="https://img.shields.io/badge/license-MIT-428f7e.svg?logo=open%20source%20initiative&logoColor=white&labelColor=555555"></a> | ||
| <a href="#tada-php-support" title="PHP Versions Supported"><img alt="PHP Versions Supported" src="https://img.shields.io/badge/php-5.3%20to%208.6-777bb3.svg?logo=php&logoColor=white&labelColor=555555"></a> | ||
| <a href="https://github.com/shivammathur/setup-php" title="GitHub action to setup PHP"><img alt="GitHub Actions status" src="https://github.com/shivammathur/setup-php/workflows/Main%20workflow/badge.svg"></a> |
There was a problem hiding this comment.
Failed Check — CI badge reverted to upstream org
This badge was previously linking to step-security/setup-php/workflows, correctly showing this fork's CI status. The sync changed it back to shivammathur/setup-php/workflows. It should be restored to point to the step-security fork:
| <a href="https://github.com/shivammathur/setup-php" title="GitHub action to setup PHP"><img alt="GitHub Actions status" src="https://github.com/shivammathur/setup-php/workflows/Main%20workflow/badge.svg"></a> | |
| <a href="https://github.com/step-security/setup-php"><img src="https://github.com/step-security/setup-php/workflows/Main%20workflow/badge.svg"></a> |
| - [Linux](https://github.com/step-security/setup-php/wiki/Dockerized-self-hosted-runner-on-Linux) | ||
| - [Windows](https://github.com/step-security/setup-php/wiki/Dockerized-self-hosted-runner-on-Windows) | ||
| - To set up a containerized self-hosted runner, refer to the following guides as per your base operating system. | ||
| - [Linux](https://github.com/shivammathur/setup-php/wiki/Dockerized-self-hosted-runner-on-Linux) |
There was a problem hiding this comment.
Failed Check — Self-hosted runner wiki links reverted to upstream org
These links previously pointed to step-security/setup-php/wiki and were changed by the sync back to shivammathur/setup-php/wiki. They should be restored to the step-security fork:
| - [Linux](https://github.com/shivammathur/setup-php/wiki/Dockerized-self-hosted-runner-on-Linux) | |
| - [Linux](https://github.com/step-security/setup-php/wiki/Dockerized-self-hosted-runner-on-Linux) | |
| - [Windows](https://github.com/step-security/setup-php/wiki/Dockerized-self-hosted-runner-on-Windows) |
|
|
||
| > Contributions are welcome! | ||
|
|
||
| - See [Contributor's Guide](.github/CONTRIBUTING.md "shivammathur/setup-php contribution guide") before you start. |
There was a problem hiding this comment.
Failed Check — .github/CONTRIBUTING.md referenced but does not exist
This line links to .github/CONTRIBUTING.md, but the file is absent from the repository. This is a broken link and will 404 for users who click it. Either create the file or remove this line until it exists.
| ) | ||
| $checksum_parts = $Checksum -split ':' | ||
| $actual_checksum = (Get-FileHash -Path $Path -Algorithm $checksum_parts[0]).Hash | ||
| return $actual_checksum -eq $checksum_parts[1] |
There was a problem hiding this comment.
Security Finding — Implicit case-insensitive comparison
Get-FileHash returns an uppercase hex digest (e.g. ABC123...), while extractChecksum() in TypeScript normalises the expected hash to lowercase before it reaches this script. PowerShell's -eq is case-insensitive for strings by default, so the comparison currently works — but this is a subtle implicit dependency.
Recommend making the intent explicit to guard against future changes:
| return $actual_checksum -eq $checksum_parts[1] | |
| return $actual_checksum.ToLower() -eq $checksum_parts[1] |
| } | ||
| const extension = await scriptExtension(os); | ||
| const src = path.dirname(path.dirname(run_path)); | ||
| const dest = fs.mkdtempSync(src + '-verbose-'); |
There was a problem hiding this comment.
Warning — Verbose temp directory is never cleaned up
mkdtempSync creates a copy of the entire scripts directory here whenever verbose mode is active. This temp directory (<src>-verbose-XXXXXX/) is returned and executed but never explicitly removed after the action finishes. The GitHub Actions runner workspace cleanup will eventually clear it, but an explicit cleanup in install.ts (e.g. fs.rmSync(dest, { recursive: true, force: true }) in a finally block after exec(tool + run_path)) would be safer and avoid accumulating temp copies on self-hosted runners across runs.
Sync verificationWarning 3 items need review: 3 files to check.
Review these
✅ 32 files: exactly upstream's version
✅ 3 files: upstream's changes alongside StepSecurity's✅ 1 file: StepSecurity-specific changes only👀 3 files: upstream and StepSecurity changed the same lines✅ 2 files: build output or lockfiles, rebuilt from source✅ 1 file: checked field by field✅ config.json checks
✅ Build checks
✅ Integrity checks
|
🔍 Cherry-Pick Verification Report📦 Upstream Changes: 📋 File-by-File Analysis:
|
PR ReviewAction TypeNode-based action — uses ✅ Passed Checks
❌ Failed Checks
|
| #### `verbose` (optional) | ||
|
|
||
| - Specify to enable verbose output. | ||
| - Accepts `true`, `false`, `v`, `vv` and `vvv`. | ||
| - By default, it is set to `false`. | ||
| - See [verbose setup](#verbose-setup) for more info. |
There was a problem hiding this comment.
verbose is not a formal action input — it's an environment variable.
This section is placed in the Inputs listing alongside php-version, extensions, etc., which implies it can be passed via with: verbose: true. However, action.yml does not define a verbose input, and the code reads it via readEnv('verbose') (an env-var lookup), not core.getInput(). The correct usage is:
env:
verbose: trueOptions to fix:
- Add
verboseas a formal input toaction.yml— the TypeScript already callsreadEnv()which checks bothINPUT_VERBOSE(set by GHA for formal inputs) and the raw env var, so no code change needed. - Move this documentation block out of the Inputs section and keep it only in the Verbose Setup section below, to avoid misleading users.
shivammathur/setup-php@2.37.2...2.40.0