feat(hyperf): add OTEL collector variants and unify hyperf-otel#2
Open
wilcorrea wants to merge 3 commits into
Open
feat(hyperf): add OTEL collector variants and unify hyperf-otel#2wilcorrea wants to merge 3 commits into
wilcorrea wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughIntroduces the complete ChangesHyperf PHP 8.3 Image and CI/CD
Sequence Diagram(s)sequenceDiagram
actor Developer
participant GitHub as GitHub Actions
participant detect as detect job
participant build as build job (matrix)
participant DockerHub as Docker Hub
Developer->>GitHub: push to main (or workflow_dispatch with target)
GitHub->>detect: run detect job
detect->>detect: resolve */latest symlinks
detect->>detect: filter dirs containing Dockerfile
detect->>detect: parse variants.yaml → build descriptors JSON
detect-->>build: pass builds JSON array
build->>build: docker/setup-buildx-action
build->>DockerHub: docker login (DOCKERHUB_USERNAME/TOKEN)
build->>DockerHub: push devitools/<image>:<tag> linux/amd64
sequenceDiagram
participant Container as Container Start
participant entrypoint as entrypoint.sh
participant pgbouncer as pgbouncer
participant supervisord as supervisord
participant hyperf as hyperf app
participant otelcol as otelcol-contrib
Container->>entrypoint: exec /entrypoint.sh
alt PGBOUNCER_ENABLED != true
entrypoint->>supervisord: exec supervisord
else PGBOUNCER_ENABLED == true
entrypoint->>entrypoint: resolve PGBOUNCER_DATABASES aliases
entrypoint->>entrypoint: write /etc/pgbouncer/pgbouncer.ini
entrypoint->>entrypoint: write /etc/supervisor.d/pgbouncer.ini
entrypoint->>entrypoint: export pgb_<alias> env vars → 127.0.0.1:6432
entrypoint->>supervisord: exec supervisord
supervisord->>pgbouncer: start pgbouncer (via drop-in)
end
supervisord->>hyperf: start php /opt/www/bin/hyperf.php (priority 1)
supervisord->>otelcol: start otelcol-contrib (priority 2)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-images.yml:
- Around line 1-17: The workflow is missing an explicit permissions block which
causes it to inherit broad repository defaults; add a top-level permissions
entry requesting the minimum required for this job (e.g. set permissions:
contents: read) so the GITHUB_TOKEN used by actions only has read access to the
repo; place this permissions block at the top-level (alongside keys like name
and on) so the workflow explicitly limits the GITHUB_TOKEN scope.
- Around line 40-45: The workflow currently leaves candidates empty when
triggered by workflow_dispatch without inputs.target because the
tj-actions/changed-files step is skipped; update the dispatch path in the
build-images job to either require inputs.target or fall back to a real diff
source: when github.event_name == "workflow_dispatch" and inputs.target is
empty, call or reuse the changed-files logic
(steps.changed.outputs.all_changed_files) or compute a diff against the default
branch (e.g., fetch origin and diff HEAD...origin/main) and populate candidates
from that output; ensure the code paths that reference inputs.target,
steps.changed.outputs.all_changed_files and candidates are adjusted so
candidates is non-empty for manual runs without a target.
- Around line 23-24: Update every GitHub Action `uses:` in the workflow to pin
to an immutable commit SHA instead of a moving tag: replace actions/checkout@v4,
tj-actions/changed-files@v44, docker/setup-buildx-action@v3,
docker/login-action@v3, and docker/build-push-action@v6 with their corresponding
OWNER/REPO@<full_sha> values (you can look up each repo’s latest stable commit
SHA) and optionally retain the human-readable tag as a trailing comment; ensure
the changes are made where those `uses:` entries appear so the workflow
references specific commit SHAs.
- Around line 37-43: The workflow is vulnerable to command injection by
interpolating GitHub expressions directly into shell blocks (candidates+=("${{
inputs.target }}"), image="${{ matrix.target }}", and for dir in ${{
steps.changed.outputs.all_changed_files }}); fix by passing these values through
the environment (use env: or GITHUB_OUTPUT to set a safe variable) and then
reading the environment vars inside the run block, validate the target/matrix
value against a strict allowlist before using it (e.g., check in a switch/if
against allowed names in the run step), and iterate changed files from a
pre-sanitized newline-separated env variable (or read from a file) rather than
unquoted expansion; update the blocks that reference candidates, inputs.target,
matrix.target, steps.changed.outputs.all_changed_files and the image assignment
to use the sanitized env vars and allowlist checks.
In `@hyperf-otel/8.3/Dockerfile`:
- Around line 13-20: Create and use an unprivileged service user in the
Dockerfile: add a non-root user (e.g., "hyperf") and group, chown the runtime
directories created by the RUN mkdir -p line (/etc/pgbouncer,
/var/log/pgbouncer, /var/run/pgbouncer, /etc/supervisor.d, /var/run/supervisor)
plus copied config files (COPY rootfs/... entries) to that user, and set USER to
that account before the ENTRYPOINT so /entrypoint.sh, otelcol-contrib, Hyperf
and optional pgbouncer run unprivileged; ensure the entrypoint still has execute
permission and any startup steps that need root (if any) are performed before
switching to the non-root user.
- Around line 7-12: The Dockerfile currently downloads
otelcol-contrib_${OTEL_COLLECTOR_VERSION}_linux_amd64.tar.gz and extracts it
without verifying integrity; update the RUN step that references
OTEL_COLLECTOR_VERSION and otelcol-contrib to fetch the release checksum (and/or
signature) from the same GitHub release, install any needed tools (sha256sum or
gnupg), verify the tarball against the published SHA256 (or verify the GPG
signature with the collector project's public key), fail the build if
verification fails, and only then extract to /usr/local/bin and remove
artifacts; ensure verification commands are added before the tar -xzf invocation
so the build aborts on mismatch.
In `@hyperf-otel/8.3/rootfs/entrypoint.sh`:
- Around line 35-41: The script writes a pgbouncer stanza even when resolved
backend variables (host, name, user, pass) are empty; change entrypoint.sh to
validate the per-alias variables (host, name, user, pass — allow port default)
before appending the stanza: if any required variable is empty, print a clear
error mentioning the alias to stderr and exit 1 instead of writing "pgb_${alias}
= ..." ; only append the stanza when the check passes. Use the existing variable
names (host, port, name, user, pass, alias) to locate and modify the block.
- Around line 18-23: The code builds a prefix from each PGBOUNCER_DATABASES
alias and passes it into eval (see resolve_prefix, the eval uses around "printf
... \"\${${prefix}_...:-}\"" and "export ${prefix}_...=..."), which is a
command-injection risk; fix by first validating each alias against a strict
whitelist (e.g., /^[A-Za-z0-9_]+$/) and aborting on any mismatch, then eliminate
eval by using safe environment lookups: construct the variable name (e.g.,
var="${prefix}_HOST") and retrieve its value with a non-eval method (use
printenv/printenv "$var" or, in bash, indirect expansion ${!var}) and assign
exports with safe shell expansions instead of eval; update resolve_prefix to
only produce validated uppercase prefixes and ensure all places that previously
used eval now use the validated prefix + safe env lookup and explicit export
commands.
In `@hyperf-otel/latest/Dockerfile`:
- Around line 13-20: Create a non-root service user and switch to it before
ENTRYPOINT: add a user (e.g., "hyperf" or "service") in the Dockerfile, chown
the runtime dirs and config/entrypoint files copied from rootfs (referencing
/etc/pgbouncer, /var/log/pgbouncer, /var/run/pgbouncer, /etc/supervisor.d,
/var/run/supervisor, /etc/otel-collector-config.yaml and /entrypoint.sh) so the
unprivileged user can write them at runtime, then set USER to that account
before the existing ENTRYPOINT ["/entrypoint.sh"]; if any initialization in
/entrypoint.sh requires root, perform that chown/setup at build time or wrap
only that portion with a drop-to-root helper, otherwise ensure the container
runs the otelcol-contrib, Hyperf and optional pgbouncer processes as the new
user.
- Around line 7-12: The Dockerfile currently downloads /tmp/otelcol.tar.gz using
OTEL_COLLECTOR_VERSION and extracts it without verification; update the RUN
sequence to fetch the published checksum (or signature) for the same release
(e.g. the SHA256 or GPG signature from the GitHub Releases for
v${OTEL_COLLECTOR_VERSION}), verify /tmp/otelcol.tar.gz before running tar -xzf
(fail the build if verification fails), and only then chmod and install
otelcol-contrib; reference the OTEL_COLLECTOR_VERSION variable,
/tmp/otelcol.tar.gz, and the otelcol-contrib binary when implementing the
verification step.
In `@hyperf-otel/latest/README.md`:
- Line 50: Replace the image tag used in the README examples: find every
occurrence of the string "devitools/hyperf-otel:8.3" (used in the Dockerfile
example lines like the FROM directive) and change it to
"devitools/hyperf-otel:latest" so the `latest` README shows the unversioned tag
in all examples.
In `@hyperf-otel/latest/rootfs/entrypoint.sh`:
- Around line 35-41: The code currently always writes a pgb_${alias} stanza even
if resolved vars (host, port, name, user, pass) are empty; update the block that
computes host/port/name/user/pass (using prefix and alias) to validate required
fields (at minimum host, name, user, pass) before appending to
/etc/pgbouncer/pgbouncer.ini: if any required variable is empty, print a clear
error naming the alias and missing fields and exit non‑zero instead of writing
the stanza; otherwise write the configured stanza as before. Ensure you
reference the same variables (host, port, name, user, pass, prefix, alias) and
fail fast when validation fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d22868b-adfd-4d97-b3d2-8c645c7964d6
📒 Files selected for processing (11)
.github/workflows/build-images.ymlhyperf-otel/8.3/Dockerfilehyperf-otel/8.3/README.mdhyperf-otel/8.3/rootfs/entrypoint.shhyperf-otel/8.3/rootfs/etc/otel-collector-config.yamlhyperf-otel/8.3/rootfs/etc/supervisord.confhyperf-otel/latest/Dockerfilehyperf-otel/latest/README.mdhyperf-otel/latest/rootfs/entrypoint.shhyperf-otel/latest/rootfs/etc/otel-collector-config.yamlhyperf-otel/latest/rootfs/etc/supervisord.conf
wilcorrea
force-pushed
the
feat/hyperf-otel-image
branch
from
a215cbc to
458b51b
Compare
wilcorrea
changed the title
Restructures hyperf/ to follow the {image}/{version} pattern used by
aws-cli, node, quasar, vue and merges the standalone hyperf-otel image
into hyperf/8.3 as a multi-stage variant.
Targets:
- hyperf -> devitools/hyperf:8.3 (lean, ~97MB, ENTRYPOINT=php)
- hyperf-otel -> devitools/hyperf:8.3-<col> (~463MB, ENTRYPOINT=supervisord)
Collectors live in hyperf/8.3/otel/collectors/*.yaml. New variants can
be added by dropping a yaml file. Two ship in this PR:
- debug (vendor-neutral, logs spans to stderr)
- google (googlecloud exporter, replaces previous gcloud-specific default)
hyperf/latest is a symlink to 8.3.
Adds .github/workflows/build-images.yml: detects changed {image}/{version}
folders on push to main, follows latest symlinks, and publishes one tag
per (version, target) combo to Docker Hub via DOCKERHUB_USERNAME and
DOCKERHUB_TOKEN secrets.
wilcorrea
force-pushed
the
feat/hyperf-otel-image
branch
from
458b51b to
b7d9de4
Compare
- workflows: declare explicit contents:read permissions and pass
GitHub-context values through env: instead of inline ${{ }}
expansion (mitigates template injection risk)
- workflow_dispatch: require target input (avoids empty matrix on
manual runs without input)
- hyperf/8.3/Dockerfile: verify otelcol-contrib tarball sha256
against the published opentelemetry-collector-releases checksums
- hyperf/8.3/otel/entrypoint.sh: whitelist PGBOUNCER_DATABASES
alias chars before eval, and fail fast when a required
POSTGRES_DB_* var is empty for an alias
Keep legacy hyperf/ untouched so devitools/hyperf is not overwritten. New multi-variant work (otel, debug, google, dev) lives under php-hyperf/8.3 and publishes to devitools/php-hyperf.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
php-hyperf/8.3/README-pt-BR.md (1)
35-60: ⚖️ Poor tradeoffDocumentation omits OTEL variants; should reflect the four published image variants.
The PR introduces four build variants—base, dev, otel-debug, and otel-google—but this README only documents dev vs. production. Users will be unaware that OTEL-instrumented variants are available for distributed tracing. Consider adding a section or expanding the comparison table to list all four variants and their use cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@php-hyperf/8.3/README-pt-BR.md` around lines 35 - 60, The documentation in the "Diferenças entre as Versões" section and comparison table only describes two image variants (dev and production) but the PR introduces four variants: base, dev, otel-debug, and otel-google. Expand the comparison table to include all four variants with their respective descriptions and use cases, ensuring that users are aware of the OTEL-instrumented variants available for distributed tracing and their specific purposes (otel-debug for debugging and otel-google for Google Cloud integration).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-images.yml:
- Around line 115-133: The manifest schema validation using yq eval is missing
type checking for the 'suffix' field. Currently it only validates that the
'suffix' key exists with has("suffix"), but does not verify that the value is
actually a string type. This causes non-string values like numbers or objects to
pass validation and later fail during tag generation at the string concatenation
step with (if (.suffix // "") == "" then "" else "-" + .suffix end). Update the
yq validation condition to additionally check that 'suffix' is a string type (or
null/empty) before allowing the entry, ensuring type safety before the tag
generation logic uses it for string operations.
In `@php-hyperf/8.3/.scripts/setup-dev.sh`:
- Around line 34-39: The curl download of the sonar-scanner archive at lines
34-35 lacks integrity verification before extraction, creating a security risk.
After the initial curl command downloads sonar-scanner.zip, add a step to
download the corresponding .sha256 checksum file from binaries.sonarsource.com
and verify the archive using sha256sum -c before proceeding to the unzip
command. Additionally, consider downloading the .asc signature file and
verifying it using gpg against the SonarSource public key with fingerprint
679F1EE92B19609DE816FDE81DB198F93525EC1A to ensure authenticity. Insert these
verification steps between the curl download and the unzip extraction.
In `@php-hyperf/8.3/.scripts/setup.sh`:
- Around line 3-18: The TIMEZONE variable is not validated before being used to
create the symlink and PHP configuration. Add validation after the TIMEZONE
variable is set (after line 3) to check if the zoneinfo file exists at
/usr/share/zoneinfo/${TIMEZONE}. If the file does not exist, output an error
message and exit the script with a non-zero status code to fail fast, preventing
the creation of broken symlinks and invalid PHP timezone configuration.
In `@php-hyperf/8.3/README-pt-BR.md`:
- Line 42: The Docker Hub image name references in the README are using the
incorrect pattern `devitools/php-hyperf` when they should follow the correct
pattern `devitools/hyperf` as defined in the PR objectives. Find and replace all
occurrences of `devitools/php-hyperf` with `devitools/hyperf` throughout the
documentation, including in build commands (like the docker build command at
line 42), image name tables, and usage examples (which appear at lines 49, 58,
59, 67, and 73 according to the comment). This ensures users will pull the
correct Docker image from Docker Hub.
---
Nitpick comments:
In `@php-hyperf/8.3/README-pt-BR.md`:
- Around line 35-60: The documentation in the "Diferenças entre as Versões"
section and comparison table only describes two image variants (dev and
production) but the PR introduces four variants: base, dev, otel-debug, and
otel-google. Expand the comparison table to include all four variants with their
respective descriptions and use cases, ensuring that users are aware of the
OTEL-instrumented variants available for distributed tracing and their specific
purposes (otel-debug for debugging and otel-google for Google Cloud
integration).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45a0d3da-a7a1-48ec-a1aa-2e9dda887ae4
📒 Files selected for processing (16)
.github/workflows/build-images.yml.github/workflows/validate.ymlphp-hyperf/8.3/.scripts/setup-dev.shphp-hyperf/8.3/.scripts/setup.shphp-hyperf/8.3/Dockerfilephp-hyperf/8.3/README-pt-BR.mdphp-hyperf/8.3/README.mdphp-hyperf/8.3/otel/collectors/debug.yamlphp-hyperf/8.3/otel/collectors/google.yamlphp-hyperf/8.3/otel/entrypoint.shphp-hyperf/8.3/otel/supervisord.confphp-hyperf/8.3/rootfs/apk/repositoriesphp-hyperf/8.3/rootfs/etc/php/php-fpm.confphp-hyperf/8.3/rootfs/etc/php/php.iniphp-hyperf/8.3/variants.yamlphp-hyperf/latest
✅ Files skipped from review due to trivial changes (2)
- php-hyperf/latest
- php-hyperf/8.3/README.md
Comment on lines
+115
to
+133
| if ! yq eval 'all_c(.target != null and .target != "" and has("suffix"))' "$manifest" | grep -qx true; then | ||
| echo "::error file=${manifest}::each entry must declare 'target' (non-empty) and 'suffix' (key required, value may be empty)" | ||
| exit 1 | ||
| fi | ||
| entries=$(yq eval -o=json "$manifest") | ||
| else | ||
| entries='[{"target":"'"$image"'","suffix":""}]' | ||
| fi | ||
|
|
||
| while IFS= read -r line; do | ||
| [ -z "$line" ] && continue | ||
| builds+=("$line") | ||
| done < <(jq -c --arg img "$image" --arg ver "$version" --arg ctx "$ctx" ' | ||
| .[] | { | ||
| image: $img, | ||
| tag: ($ver + (if (.suffix // "") == "" then "" else "-" + .suffix end)), | ||
| context: $ctx, | ||
| target: .target, | ||
| build_args: ((.args // {}) | to_entries | map(.key + "=" + (.value | tostring)) | join("\n")) |
There was a problem hiding this comment.
Harden manifest schema validation for suffix type before tag generation.
At Line 115, suffix is only checked for key presence, but at Line 130 it’s treated as a string. A non-string suffix (e.g., number/object) will pass this guard and fail matrix generation.
💡 Suggested fix
- if ! yq eval 'all_c(.target != null and .target != "" and has("suffix"))' "$manifest" | grep -qx true; then
- echo "::error file=${manifest}::each entry must declare 'target' (non-empty) and 'suffix' (key required, value may be empty)"
+ if ! yq eval 'all_c(
+ (.target | type) == "!!str" and
+ .target != "" and
+ has("suffix") and
+ ((.suffix == null) or ((.suffix | type) == "!!str"))
+ )' "$manifest" | grep -qx true; then
+ echo "::error file=${manifest}::each entry must declare 'target' (non-empty string) and 'suffix' (string or null)"
exit 1
fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-images.yml around lines 115 - 133, The manifest
schema validation using yq eval is missing type checking for the 'suffix' field.
Currently it only validates that the 'suffix' key exists with has("suffix"), but
does not verify that the value is actually a string type. This causes non-string
values like numbers or objects to pass validation and later fail during tag
generation at the string concatenation step with (if (.suffix // "") == "" then
"" else "-" + .suffix end). Update the yq validation condition to additionally
check that 'suffix' is a string type (or null/empty) before allowing the entry,
ensuring type safety before the tag generation logic uses it for string
operations.
Comment on lines
+34
to
+39
| curl -fSL https://binaries.sonarsource.com/Distribution/sonar-scanner-cli/sonar-scanner-cli-${SONAR_SCANNER_VERSION}-linux-x64.zip \ | ||
| -o /opt/sonar-scanner.zip | ||
|
|
||
| unzip -qq /opt/sonar-scanner.zip -d /opt | ||
| mv /opt/sonar-scanner-${SONAR_SCANNER_VERSION}-linux-x64 /sonar-scanner | ||
| rm /opt/sonar-scanner.zip |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
SonarScanner official releases checksum verification signature
💡 Result:
SonarScanner release binaries can be verified using the checksum and signature files provided alongside the main archive at the SonarSource distribution CDN [1]. For most SonarSource binary distributions, the following files are available in the same directory as the main archive (e.g.,.zip) [1]: - Checksum files:.md5,.sha1, and.sha256 files are typically provided to allow for integrity verification of the downloaded binary [1]. - Signature files: A detached OpenPGP signature file (typically with a.asc extension) is provided to verify the authenticity of the archive [1]. Recent developments have improved the automation of this process, particularly for CI/CD environments. For example, the official SonarQube GitHub Action for scanning was updated to include automated OpenPGP signature verification, ensuring that downloaded binaries are validated against the SonarSource public key before use [2]. This implementation uses the SonarSource public key fingerprint: 679F1EE92B19609DE816FDE81DB198F93525EC1A [2]. If you are performing manual verification: 1. Download the binary archive and its corresponding.asc and.sha256 files from the official SonarSource distribution server [1]. 2. Use the provided.sha256 file to verify that the file has not been corrupted during download [3][1]. 3. Use the.asc file and the official SonarSource public key to verify that the file is authentic [2][1]. If you encounter issues such as a mismatch, it is recommended to re-download the file to ensure the transfer was not interrupted or corrupted [4][1]. Note that SonarScanner binaries themselves perform internal checksum validation for components they download (like the JRE) as part of their standard operation [5][6].
Citations:
- 1: https://community.sonarsource.com/t/broken-sonarqube-download-checksums/64697
- 2: SQSCANGHA-140 Add OpenPGP signature verification for scanner downloads SonarSource/sonarqube-scan-action#235
- 3: https://oss.anchore.com/docs/installation/verification/
- 4: https://community.sonarsource.com/t/problem-downloading-of-sonarqube-8-9-6-lts-zip/57619
- 5: Add support for JRE provisioning: Jre download checksum validation SonarSource/sonar-scanner-msbuild#2016
- 6: Add support for JRE provisioning: Delete archive when checksum validation fails. SonarSource/sonar-scanner-msbuild#2033
🌐 Web query:
sonarsource SonarScanner CLI release integrity verification
💡 Result:
To verify the integrity of SonarScanner CLI releases and related components, SonarSource employs different methods depending on the integration and environment. Recent security enhancements have introduced explicit OpenPGP signature verification for SonarScanner CLI downloads, particularly within automated CI/CD workflows [1]. As of April 2026, the SonarSource GitHub Action (sonarqube-scan-action) was updated to perform OpenPGP signature verification by default when downloading scanner binaries [1][2]. This process fetches a detached.asc signature file and validates it against the official SonarSource public key (Fingerprint: 679F1EE92B19609DE816FDE81DB18F93525EC1A) [1]. For other integrations, integrity verification methods include: 1. Checksum Validation: Many SonarSource tools, such as the npm-based scanner and MSBuild scanner, implement internal SHA-256 checksum validation to ensure the integrity of downloaded binaries or JRE components [3][4]. 2. Build-Time Dependency Verification: When using build tools like Gradle, SonarSource plugins rely on standard dependency verification mechanisms (e.g., verification-metadata.xml) where SHA-256 hashes are used to confirm the authenticity of artifacts retrieved from repositories like Maven Central [5]. 3. GitHub Release Signing: Official releases on GitHub are created and signed with verified signatures, which can be audited via the GitHub interface [6]. It is important to note that for macOS AArch64 users, the SonarScanner CLI binary is not currently Apple-verified, which may trigger security warnings [7][8]. Official documentation provides a workaround using xattr to bypass the quarantine flag, which should only be used if the binary source is trusted [7][9]. Users are encouraged to utilize official SonarSource distribution channels to minimize supply chain risks [10].
Citations:
- 1: SQSCANGHA-140 Add OpenPGP signature verification for scanner downloads SonarSource/sonarqube-scan-action#235
- 2: SonarSource/sonarqube-scan-action@55e4480
- 3: Add support for JRE provisioning: Jre download checksum validation SonarSource/sonar-scanner-msbuild#2016
- 4: https://github.com/SonarSource/sonar-scanner-npm/blob/06936b1b/src/file.ts
- 5: https://github.com/SonarSource/sonar-scanner-gradle/tree/refs/heads/master
- 6: https://github.com/SonarSource/sonar-scanner-cli/releases
- 7: https://docs.sonarsource.com/sonarqube-server/analyzing-source-code/scanners/sonarscanner
- 8: https://docs.sonarsource.com/sonarqube-cloud/analyzing-source-code/scanners/sonarscanner-cli
- 9: https://docs.sonarsource.com/sonarqube-server/2026.1/analyzing-source-code/scanners/sonarscanner
- 10: ci: sonarqube-scan-action downloads Sonar Scanner CLI without verifying argoproj/argo-cd#27079
Verify SonarScanner archive integrity before extraction.
The download at lines 34-35 is extracted without checksum or signature verification. SonarSource provides .sha256 checksum files and .asc OpenPGP signature files alongside releases at binaries.sonarsource.com for exactly this purpose.
Add verification using the provided checksum before unzip (e.g., using sha256sum -c with the corresponding .sha256 file). Signature verification against the official SonarSource public key (fingerprint: 679F1EE92B19609DE816FDE81DB198F93525EC1A) is also recommended to verify authenticity, as implemented in recent SonarQube GitHub Actions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@php-hyperf/8.3/.scripts/setup-dev.sh` around lines 34 - 39, The curl download
of the sonar-scanner archive at lines 34-35 lacks integrity verification before
extraction, creating a security risk. After the initial curl command downloads
sonar-scanner.zip, add a step to download the corresponding .sha256 checksum
file from binaries.sonarsource.com and verify the archive using sha256sum -c
before proceeding to the unzip command. Additionally, consider downloading the
.asc signature file and verifying it using gpg against the SonarSource public
key with fingerprint 679F1EE92B19609DE816FDE81DB198F93525EC1A to ensure
authenticity. Insert these verification steps between the curl download and the
unzip extraction.
Comment on lines
+3
to
+18
| TIMEZONE=${1:-UTC} | ||
|
|
||
| git config --global --add safe.directory /opt/www | ||
| git config --global init.defaultBranch main | ||
|
|
||
| # - config PHP | ||
| { | ||
| echo "upload_max_filesize=128M" | ||
| echo "post_max_size=128M" | ||
| echo "memory_limit=1G" | ||
| echo "date.timezone=${TIMEZONE}" | ||
| } | tee /etc/php83/conf.d/zzz_0_php.ini | ||
|
|
||
| # - config timezone | ||
| ln -sf "/usr/share/zoneinfo/${TIMEZONE}" /etc/localtime | ||
| echo "${TIMEZONE}" > /etc/timezone |
There was a problem hiding this comment.
Validate timezone input before applying it.
If TIMEZONE is invalid, Line 17 still creates a broken symlink and PHP gets an invalid timezone value. Fail fast when the zoneinfo file is missing.
💡 Suggested fix
TIMEZONE=${1:-UTC}
+
+if [ ! -f "/usr/share/zoneinfo/${TIMEZONE}" ]; then
+ echo "Error: invalid timezone '${TIMEZONE}'" >&2
+ exit 1
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TIMEZONE=${1:-UTC} | |
| git config --global --add safe.directory /opt/www | |
| git config --global init.defaultBranch main | |
| # - config PHP | |
| { | |
| echo "upload_max_filesize=128M" | |
| echo "post_max_size=128M" | |
| echo "memory_limit=1G" | |
| echo "date.timezone=${TIMEZONE}" | |
| } | tee /etc/php83/conf.d/zzz_0_php.ini | |
| # - config timezone | |
| ln -sf "/usr/share/zoneinfo/${TIMEZONE}" /etc/localtime | |
| echo "${TIMEZONE}" > /etc/timezone | |
| TIMEZONE=${1:-UTC} | |
| if [ ! -f "/usr/share/zoneinfo/${TIMEZONE}" ]; then | |
| echo "Error: invalid timezone '${TIMEZONE}'" >&2 | |
| exit 1 | |
| fi | |
| git config --global --add safe.directory /opt/www | |
| git config --global init.defaultBranch main | |
| # - config PHP | |
| { | |
| echo "upload_max_filesize=128M" | |
| echo "post_max_size=128M" | |
| echo "memory_limit=1G" | |
| echo "date.timezone=${TIMEZONE}" | |
| } | tee /etc/php83/conf.d/zzz_0_php.ini | |
| # - config timezone | |
| ln -sf "/usr/share/zoneinfo/${TIMEZONE}" /etc/localtime | |
| echo "${TIMEZONE}" > /etc/timezone |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@php-hyperf/8.3/.scripts/setup.sh` around lines 3 - 18, The TIMEZONE variable
is not validated before being used to create the symlink and PHP configuration.
Add validation after the TIMEZONE variable is set (after line 3) to check if the
zoneinfo file exists at /usr/share/zoneinfo/${TIMEZONE}. If the file does not
exist, output an error message and exit the script with a non-zero status code
to fail fast, preventing the creation of broken symlinks and invalid PHP
timezone configuration.
| Inclui **Xdebug**, **PCOV** e **Sonar Scanner** para depuração e análise de código. | ||
|
|
||
| ```sh | ||
| docker build --platform="linux/amd64" --build-arg APP_TARGET=dev -t devitools/php-hyperf:8.3-dev . |
There was a problem hiding this comment.
Docker Hub image name is incorrect across the documentation.
The PR objectives state that the pushed Docker Hub image tags will follow the pattern devitools/hyperf:{version}, but this README documents the image as devitools/php-hyperf:8.3-dev and devitools/php-hyperf:8.3. This mismatch will cause users to pull the wrong or non-existent image.
Replace all occurrences of devitools/php-hyperf with devitools/hyperf in the build commands, table, and usage examples.
🐛 Proposed fixes
-docker build --platform="linux/amd64" --build-arg APP_TARGET=dev -t devitools/php-hyperf:8.3-dev .
+docker build --platform="linux/amd64" --build-arg APP_TARGET=dev -t devitools/hyperf:8.3-dev .-docker build --platform="linux/amd64" -t devitools/php-hyperf:8.3 .
+docker build --platform="linux/amd64" -t devitools/hyperf:8.3 . | Versão | Recursos Incluídos |
|-----------------------|------------------|
-| `devitools/php-hyperf:8.3-dev` | PHP 8.3 + Xdebug + PCOV + Sonar Scanner |
-| `devitools/php-hyperf:8.3` | PHP 8.3 otimizado para produção |
+| `devitools/hyperf:8.3-dev` | PHP 8.3 + Xdebug + PCOV + Sonar Scanner |
+| `devitools/hyperf:8.3` | PHP 8.3 otimizado para produção |-docker run --rm -it devitools/php-hyperf:8.3-dev php -v
+docker run --rm -it devitools/hyperf:8.3-dev php -v-docker run --rm -it -v $(pwd):/opt/www devitools/php-hyperf:8.3-dev composer create-project hyperf/hyperf-skeleton .
+docker run --rm -it -v $(pwd):/opt/www devitools/hyperf:8.3-dev composer create-project hyperf/hyperf-skeleton .Also applies to: 49-49, 58-58, 59-59, 67-67, 73-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@php-hyperf/8.3/README-pt-BR.md` at line 42, The Docker Hub image name
references in the README are using the incorrect pattern `devitools/php-hyperf`
when they should follow the correct pattern `devitools/hyperf` as defined in the
PR objectives. Find and replace all occurrences of `devitools/php-hyperf` with
`devitools/hyperf` throughout the documentation, including in build commands
(like the docker build command at line 42), image name tables, and usage
examples (which appear at lines 49, 58, 59, 67, and 73 according to the
comment). This ensures users will pull the correct Docker image from Docker Hub.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hyperf/to follow the{image}/{version}pattern used byaws-cli,node,quasar,vue.hyperf-otel/folder intohyperf/8.3/as a multi-stage variant — same image, different--target.variants.yamlmanifest per{image}/{version}that declares which tags to build (target + suffix + optional build-args). Replaces image-specific scanning conventions.hyperf/latestis a symlink tohyperf/8.3.build-images.yml: on push tomain, readsvariants.yamlof changed{image}/{version}folders (followslatestsymlinks) and publishes one Docker Hub tag per entry.validate.yml: onpull_request, validates structure of every changedvariants.yaml.Why
The previous
hyperf-otelimage bundled agooglecloud-specific OTEL config that wouldn't fit AWS/Azure/on-prem consumers. Making the collector and dev-tools toggles configurable via build-args + publishing multiple variants lets consumers pick the right one atFROMtime without rebuilding the base. Merging back intohyperf/removes duplication; the OTEL layer becomes an opt-in second stage.Variant manifest format
Rules:
target(non-empty) andsuffix(key required; empty value = plain:<version>tag).args(optional) is a mapping passed as--build-arg KEY=VALUEto docker build.<image>:<version>if suffix empty, else<image>:<version>-<suffix>.variants.yamlget a single plain build (<image>:<version>from default Dockerfile target). Backwards-compatible withaws-cli,node, etc.Tags produced from this PR
hyperfdevitools/hyperf:8.3hyperfAPP_TARGET=devdevitools/hyperf:8.3-devhyperf-otelCOLLECTOR=debugdevitools/hyperf:8.3-otelhyperf-otelCOLLECTOR=googledevitools/hyperf:8.3-googlePlus matching
:latest,:latest-dev,:latest-otel,:latest-googlevia the symlink (workflow detects the symlink and includes it in the matrix when the canonical version changes).Workflows
build-images.yml(on push to main)*/*/Dockerfile,*/*/variants.yaml,*/*/rootfs/**,*/*/.scripts/**,*/*/otel/**, or*/latest.{image}/{version}dir, followslatestsymlinks pointing to it.(image, version): readsvariants.yamlviayq → jq(both pre-installed on Ubuntu runners) and emits one matrix entry per variant.docker/build-push-action@v6call (linux/amd64) withtargetandbuild-argsderived from the manifest. GHA cache scoped per tag.validate.yml(on PRs)*/*/variants.yaml.yq).targetand thesuffixkey (value may be empty).argsis a mapping when present.DockerfiledeclaresFROM ... AS <target>for every referenced target.Adding new variants
Drop a row in
<image>/<version>/variants.yaml. The build-images workflow picks it up on the next push to main. The validate workflow catches malformed entries during the PR.Examples:
{ target: hyperf-otel, suffix: jaeger, args: { COLLECTOR: jaeger } }(with matchinghyperf/8.3/otel/collectors/jaeger.yaml).{ target: hyperf, suffix: arm, args: { TARGETPLATFORM: linux/arm64 } }.Local validation
docker buildwith appropriate--targetand--build-arg.otelcol-contrib/supervisord— backwards compatible with currentdevitools/hyperf:8.3consumers.validate.ymllogic smoke-tested locally: catches missing target/suffix, invalid args type, missing Dockerfile stage, and duplicate suffix tags.yq → jqparser produces the expected 4 matrix entries from the manifest.Required secrets
DOCKERHUB_USERNAME✅ configuredDOCKERHUB_TOKEN✅ configuredTest plan
validate.yml; passes on this branch (manifest is valid)build-images.ymlhyperf/8.3× 4 variants +hyperf/latest× 4 variants):8.3and:latest,:8.3-devand:latest-dev, etc.docker pull devitools/hyperf:8.3(lean) is ~97MB withphpentrypointdocker pull devitools/hyperf:8.3-otelis ~463MB with/entrypoint.sh(supervisord)Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores