Add PHP 8.5 support#14
Merged
SilverFire merged 5 commits intomasterfrom Apr 20, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds PHP 8.5 build artifacts (Makefile, CLI and FPM Alpine Dockerfiles, FPM entrypoint) and applies multiple small fixes: sed delimiter fixes and comment corrections across existing entrypoints, plus FPM Changes
Sequence Diagram(s)sequenceDiagram
participant Container
participant Entrypoint as "hiqdev-docker-entrypoint"
participant NewRelic as "New Relic config"
participant Xdebug as "Xdebug ini"
participant PHPFPM as "php-fpm"
Container->>Entrypoint: container start (env)
Entrypoint->>NewRelic: if NEW_RELIC_LICENSE_KEY -> enable & set key/appname/daemon
Entrypoint->>Xdebug: if ENV != prod and PHP_XDEBUG_ENABLED != 0 -> enable ini
Entrypoint->>Entrypoint: auto-detect XDEBUG_CLIENT_HOST (host.docker.internal or gateway)
Entrypoint->>Xdebug: write xdebug.client_host / port / start_with_request
Entrypoint->>PHPFPM: normalize args (prepend php-fpm if needed)
Entrypoint->>PHPFPM: exec command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 9
🧹 Nitpick comments (1)
src/8.5/fpm-alpine/hiqdev-docker-entrypoint (1)
5-5: Quote the command substitution.
postconf -e myhostname=$(hostname)is unquoted and also passes themyhostname=...as a single argument composed by shell-splitting. Ifhostnameever returns empty or odd output,postconfgets an empty/invalid setting. Prefer quoting:-postconf -e myhostname=$(hostname) +postconf -e "myhostname=$(hostname)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/hiqdev-docker-entrypoint` at line 5, The postconf invocation uses an unquoted command substitution (postconf -e myhostname=$(hostname)) which can lead to word-splitting or an empty/invalid value; fix by quoting the substitution so the entire assignment is passed as a single argument (e.g., use a quoted form of myhostname=$(hostname)) in the same postconf -e invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/8.5/cli-alpine/Dockerfile`:
- Line 104: The apk installation line that fetches gnu-libiconv using an
unpinned repo and --allow-untrusted is a supply-chain and reproducibility risk;
update the apk add invocation that references $GNU_LIBICONV_RELEASE/gnu-libiconv
to use a versioned Alpine community repo (e.g., alpine/v3.20/community) instead
of …/latest-stable/community and remove --allow-untrusted, and instead import
and add the Alpine community signing key before the apk add so the package
signature is verified; apply the same change pattern to the parallel apk
invocation that installs gnu-libiconv in the other Dockerfile variant.
- Line 124: Replace the invalid PHP ini key opcache_revalidate_freq with the
correct opcache.revalidate_freq in the Dockerfile echo lines (the occurrence
shown and the duplicate in the other Dockerfile) so PHP reads the directive, and
ensure the hiqdev-docker-entrypoint sed pattern (which expects
opcache.revalidate_freq) will match after this change; update both Dockerfile
occurrences of the underscore key to the dotted key.
In `@src/8.5/fpm-alpine/Dockerfile`:
- Around line 114-119: The Dockerfile currently downloads and extracts the New
Relic tarball with curl|tar and runs /tmp/newrelic-install without integrity
checks, and installs gnu-libiconv with --allow-untrusted; fix by verifying
artifacts: add a pinned SHA256 digest variable for NEW_RELIC_AGENT_VERSION and
fetch the corresponding .sha256 (or hardcode the digest) and verify the archive
before extracting and running NR_INSTALL (referencing NEW_RELIC_AGENT_VERSION
and /tmp/newrelic-php5-*), and for gnu-libiconv stop using --allow-untrusted by
pinning the Alpine community repo to a specific release and importing the
repository signing key before apk add gnu-libiconv (referencing gnu-libiconv and
the apk add command).
- Line 1: The Dockerfile currently pulls the wrong base image (it uses
"php:8.4-fpm-alpine") so the fpm variant does not actually provide PHP 8.5;
update the FROM line in the Dockerfile under src/8.5/fpm-alpine to use
"php:8.5-fpm-alpine" instead, verify the image tag referenced by the Makefile
(hiqdev/php:8.5-fpm-alpine) matches this change, and run a quick build to
confirm the resulting image reports PHP 8.5.
- Line 139: The Dockerfile writes an invalid PHP ini key using an underscore
("opcache_revalidate_freq") which prevents the entrypoint script
hiqdev-docker-entrypoint (sed on the dotted key) from matching; update the
echoed setting in the fpm-alpine Dockerfile from opcache_revalidate_freq to the
correct opcache.revalidate_freq so the generated ini uses the dotted key and the
sed in hiqdev-docker-entrypoint will successfully find and replace it in dev
mode.
- Around line 163-172: The current startup block appends a [global] section into
the pool file (www.conf) which is ignored; instead write the global directives
into the master config and keep only the pool section in the pool file. Modify
the script that currently writes to /usr/local/etc/php-fpm.d/www.conf so that
it: 1) appends the "[www]" block and pool-specific lines (including
pm.max_requests = $FPM_MAX_REQUESTS and catch_workers_output = yes) to
/usr/local/etc/php-fpm.d/www.conf, and 2) appends the "[global]" header and the
global directives (process.max = $FPM_PROCESS_MAX, emergency_restart_threshold,
emergency_restart_interval, process_control_timeout) to
/usr/local/etc/php-fpm.conf; ensure you remove any "[global]" lines from the
pool file to avoid php-fpm warnings.
In `@src/8.5/fpm-alpine/hiqdev-docker-entrypoint`:
- Around line 27-29: Fix the typo in the comment by changing "Diable Opcache" to
a clearer "Disable opcache revalidation caching" (the log/message "[ opcache ]
Configuring dev mode" can remain), and update the sed invocation that targets
opcache.revalidate_freq so it matches the actual ini key used elsewhere
(opcache_revalidate_freq) — either change the sed target to
opcache_revalidate_freq or make the regex accept both dot and underscore
variants (e.g., allow [._] between words) so the replacement reliably sets
revalidation to 0; ensure the sed command string and the described intent
("disable revalidation caching") stay consistent with the Dockerfile/config that
sets the ini key.
- Line 63: The sed substitution for xdebug.start_with_request is missing the
closing '/' so the s/// is malformed and causes sed to fail under set -e; update
the sed command that references XDEBUG_START_WITH_REQUEST (the line containing
sed -i
"s/xdebug.start_with_request\s*=\s*.*/xdebug.start_with_request=$XDEBUG_START_WITH_REQUEST"
...) to include the trailing '/' before the closing quote so it matches the
correct form used on the other lines (lines that handle XDEBUG_MODE and
XDEBUG_CLIENT_HOST).
- Line 34: The sed invocation that uncomments blackfire.ini uses a broad pattern
(sed -i "s/^;//" /usr/local/etc/php/conf.d/blackfire.ini) which will strip
leading semicolons on every line; change it to use a line-address restriction
like the xdebug handler (use the first-line-only form, e.g. 1s/^;//) so only the
intended commented extension line is uncommented; update the sed command that
targets blackfire.ini accordingly.
---
Nitpick comments:
In `@src/8.5/fpm-alpine/hiqdev-docker-entrypoint`:
- Line 5: The postconf invocation uses an unquoted command substitution
(postconf -e myhostname=$(hostname)) which can lead to word-splitting or an
empty/invalid value; fix by quoting the substitution so the entire assignment is
passed as a single argument (e.g., use a quoted form of myhostname=$(hostname))
in the same postconf -e invocation.
🪄 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: 8bc97ae2-e8c4-4ad4-a4e6-97c574eb5cdc
📒 Files selected for processing (4)
src/8.5/Makefilesrc/8.5/cli-alpine/Dockerfilesrc/8.5/fpm-alpine/Dockerfilesrc/8.5/fpm-alpine/hiqdev-docker-entrypoint
| # Sockets | ||
| && docker-php-ext-install sockets \ | ||
|
|
||
| && apk add gnu-libiconv="$GNU_LIBICONV_RELEASE" --update-cache --repository http://dl-cdn.alpinelinux.org/alpine/latest-stable/community/ --allow-untrusted \ |
There was a problem hiding this comment.
Unpinned Alpine repo + --allow-untrusted is fragile and a supply-chain risk.
Pulling gnu-libiconv from …/alpine/latest-stable/community/ with --allow-untrusted means:
- The repo moves as Alpine releases new stable versions, so a future rebuild against a newer Alpine base may fail to resolve the pinned
$GNU_LIBICONV_RELEASE=1.18-r0(reproducibility risk). --allow-untrusteddisables signature verification for that package.
Consider pinning the repo to the matching Alpine version (e.g., alpine/v3.20/community/) and importing the community signing key instead of --allow-untrusted. This mirrors what src/8.5/fpm-alpine/Dockerfile line 114 also does, so apply both places.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/cli-alpine/Dockerfile` at line 104, The apk installation line that
fetches gnu-libiconv using an unpinned repo and --allow-untrusted is a
supply-chain and reproducibility risk; update the apk add invocation that
references $GNU_LIBICONV_RELEASE/gnu-libiconv to use a versioned Alpine
community repo (e.g., alpine/v3.20/community) instead of
…/latest-stable/community and remove --allow-untrusted, and instead import and
add the Alpine community signing key before the apk add so the package signature
is verified; apply the same change pattern to the parallel apk invocation that
installs gnu-libiconv in the other Dockerfile variant.
| echo "opcache.enable=1"; \ | ||
| echo "opcache.memory_consumption=1024"; \ | ||
| echo "opcache.max_accelerated_files=9000"; \ | ||
| echo "opcache_revalidate_freq=120"; \ |
There was a problem hiding this comment.
Typo: opcache_revalidate_freq should be opcache.revalidate_freq.
This writes an invalid ini directive (underscore instead of dot), so PHP silently ignores it and falls back to the default opcache.revalidate_freq=2. Any operator expecting a 120s revalidation interval in production will instead get 2s.
Note: this same typo also exists in src/8.5/fpm-alpine/Dockerfile at line 139, and it additionally breaks the entrypoint script's sed at hiqdev-docker-entrypoint line 29 which searches for opcache.revalidate_freq (with the dot) and won't find anything to replace.
🔧 Proposed fix
- echo "opcache_revalidate_freq=120"; \
+ echo "opcache.revalidate_freq=120"; \📝 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
| echo "opcache_revalidate_freq=120"; \ | |
| echo "opcache.revalidate_freq=120"; \ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/cli-alpine/Dockerfile` at line 124, Replace the invalid PHP ini key
opcache_revalidate_freq with the correct opcache.revalidate_freq in the
Dockerfile echo lines (the occurrence shown and the duplicate in the other
Dockerfile) so PHP reads the directive, and ensure the hiqdev-docker-entrypoint
sed pattern (which expects opcache.revalidate_freq) will match after this
change; update both Dockerfile occurrences of the underscore key to the dotted
key.
Comment on lines
+114
to
+119
| && apk add gnu-libiconv=$GNU_LIBICONV_RELEASE --update-cache --repository http://dl-cdn.alpinelinux.org/alpine/latest-stable/community/ --allow-untrusted \ | ||
|
|
||
| # New Relic | ||
| && curl -L https://download.newrelic.com/php_agent/archive/$NEW_RELIC_AGENT_VERSION/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz | tar -C /tmp -zx \ | ||
| && NR_INSTALL_USE_CP_NOT_LN=1 NR_INSTALL_SILENT=1 /tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl/newrelic-install install \ | ||
| && rm -rf /tmp/newrelic-php5-* /tmp/nrinstall* \ |
There was a problem hiding this comment.
New Relic tarball is downloaded without integrity verification.
The agent archive is pulled via curl -L and piped straight into tar, and then the newrelic-install binary it contains is executed as root at build time. A MITM or an upstream compromise would silently yield arbitrary code execution inside the image. Consider either:
- Verifying a SHA256 of the archive against a pinned digest before extracting, or
- Using New Relic's official signed package (apk with their signing key) if available.
Same goes for the gnu-libiconv install at line 114 using --allow-untrusted — pin the repo to a specific Alpine version and import the community signing key.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/fpm-alpine/Dockerfile` around lines 114 - 119, The Dockerfile
currently downloads and extracts the New Relic tarball with curl|tar and runs
/tmp/newrelic-install without integrity checks, and installs gnu-libiconv with
--allow-untrusted; fix by verifying artifacts: add a pinned SHA256 digest
variable for NEW_RELIC_AGENT_VERSION and fetch the corresponding .sha256 (or
hardcode the digest) and verify the archive before extracting and running
NR_INSTALL (referencing NEW_RELIC_AGENT_VERSION and /tmp/newrelic-php5-*), and
for gnu-libiconv stop using --allow-untrusted by pinning the Alpine community
repo to a specific release and importing the repository signing key before apk
add gnu-libiconv (referencing gnu-libiconv and the apk add command).
| echo "opcache.enable=1"; \ | ||
| echo "opcache.memory_consumption=1024"; \ | ||
| echo "opcache.max_accelerated_files=9000"; \ | ||
| echo "opcache_revalidate_freq=120"; \ |
There was a problem hiding this comment.
Same opcache_revalidate_freq underscore typo as the cli-alpine Dockerfile.
See the detailed comment on src/8.5/cli-alpine/Dockerfile line 124 — the valid PHP ini key is opcache.revalidate_freq (with a dot). Additionally, this typo causes the entrypoint's sed at hiqdev-docker-entrypoint line 29 to silently no-op in dev mode because it searches for the dotted form that is never written.
🔧 Proposed fix
- echo "opcache_revalidate_freq=120"; \
+ echo "opcache.revalidate_freq=120"; \📝 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
| echo "opcache_revalidate_freq=120"; \ | |
| echo "opcache.revalidate_freq=120"; \ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/fpm-alpine/Dockerfile` at line 139, The Dockerfile writes an invalid
PHP ini key using an underscore ("opcache_revalidate_freq") which prevents the
entrypoint script hiqdev-docker-entrypoint (sed on the dotted key) from
matching; update the echoed setting in the fpm-alpine Dockerfile from
opcache_revalidate_freq to the correct opcache.revalidate_freq so the generated
ini uses the dotted key and the sed in hiqdev-docker-entrypoint will
successfully find and replace it in dev mode.
Comment on lines
+27
to
+29
| # Diable Opcache | ||
| echo "[ opcache ] Configuring dev mode" | ||
| sed -i "s/opcache.revalidate_freq\s*=\s*[[:digit:]]\+/opcache.revalidate_freq=0/" /usr/local/etc/php/conf.d/docker-php-ext-opcache.ini |
There was a problem hiding this comment.
Minor: typo and dead opcache sed due to upstream key mismatch.
- Line 27 comment says "Diable Opcache" — should be "Disable" (and the actual intent is to disable revalidation caching, not opcache itself).
- Line 29 targets
opcache.revalidate_freq(dot), butsrc/8.5/fpm-alpine/Dockerfileline 139 writesopcache_revalidate_freq(underscore). Once the Dockerfile typo is fixed this sed will start working; flagging together to keep them in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/fpm-alpine/hiqdev-docker-entrypoint` around lines 27 - 29, Fix the
typo in the comment by changing "Diable Opcache" to a clearer "Disable opcache
revalidation caching" (the log/message "[ opcache ] Configuring dev mode" can
remain), and update the sed invocation that targets opcache.revalidate_freq so
it matches the actual ini key used elsewhere (opcache_revalidate_freq) — either
change the sed target to opcache_revalidate_freq or make the regex accept both
dot and underscore variants (e.g., allow [._] between words) so the replacement
reliably sets revalidation to 0; ensure the sed command string and the described
intent ("disable revalidation caching") stay consistent with the
Dockerfile/config that sets the ini key.
| # Enable Blackfire | ||
| if [ "$PHP_BLACKFIRE_ENABLED" = "1" ]; then | ||
| echo "[ blackfire ] Enabling Blackfire" | ||
| sed -i "s/^;//" /usr/local/etc/php/conf.d/blackfire.ini |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how blackfire.ini is produced (if at all) in this repo — the Dockerfile does not create it,
# so it's probably shipped by an upstream layer or external install. Confirm that only the first line needs to be uncommented.
fd -HI 'blackfire' | head -50
rg -nP --type=dockerfile -C2 'blackfire'Repository: hiqdev/docker-php
Length of output: 95
🏁 Script executed:
# Find the entrypoint file
fd -i 'hiqdev-docker-entrypoint' | head -20Repository: hiqdev/docker-php
Length of output: 590
🏁 Script executed:
# Once found, examine the context around lines 30-45
head -50 "src/8.5/fpm-alpine/hiqdev-docker-entrypoint" | tail -25Repository: hiqdev/docker-php
Length of output: 1085
🏁 Script executed:
# Look for blackfire.ini references in the repo
rg -i 'blackfire' | head -30Repository: hiqdev/docker-php
Length of output: 3007
🏁 Script executed:
# Verify xdebug.ini structure if available in repo
rg -i 'docker-php-ext-xdebug' | head -20Repository: hiqdev/docker-php
Length of output: 3000
🏁 Script executed:
# Check other versions to see if pattern is consistent
grep -n "sed.*^;" src/*/fpm*/hiqdev-docker-entrypoint | head -30Repository: hiqdev/docker-php
Length of output: 2486
Use line-address restriction to match xdebug pattern.
The sed command s/^;// will uncomment any line starting with ; in the file. While blackfire.ini currently contains only one commented line (extension=blackfire.so), this pattern lacks the defensive clarity of the xdebug handler at line 40, which correctly restricts to the first line using 1s/^;//. For consistency and robustness against future config changes, apply the same restriction.
🔧 Proposed fix
- sed -i "s/^;//" /usr/local/etc/php/conf.d/blackfire.ini
+ sed -i "1s/^;//" /usr/local/etc/php/conf.d/blackfire.ini📝 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
| sed -i "s/^;//" /usr/local/etc/php/conf.d/blackfire.ini | |
| sed -i "1s/^;//" /usr/local/etc/php/conf.d/blackfire.ini |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/fpm-alpine/hiqdev-docker-entrypoint` at line 34, The sed invocation
that uncomments blackfire.ini uses a broad pattern (sed -i "s/^;//"
/usr/local/etc/php/conf.d/blackfire.ini) which will strip leading semicolons on
every line; change it to use a line-address restriction like the xdebug handler
(use the first-line-only form, e.g. 1s/^;//) so only the intended commented
extension line is uncommented; update the sed command that targets blackfire.ini
accordingly.
Move [global] section (process.max, emergency_restart_*, process_control_timeout) from www.conf to php-fpm.conf where it is actually parsed by PHP-FPM. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/8.5/fpm-alpine/Dockerfile (2)
137-137:⚠️ Potential issue | 🔴 CriticalUse the dotted opcache directive name.
opcache_revalidate_freqis not the directive configured by the entrypoint;src/8.5/fpm-alpine/hiqdev-docker-entrypointeditsopcache.revalidate_freq, so dev-mode startup currently no-ops for this setting.🐛 Proposed fix
- echo "opcache_revalidate_freq=120"; \ + echo "opcache.revalidate_freq=120"; \Run this read-only check to verify the Dockerfile and entrypoint use the same key:
#!/bin/bash rg -n 'opcache_revalidate_freq|opcache\.revalidate_freq' \ src/8.5/fpm-alpine/Dockerfile \ src/8.5/fpm-alpine/hiqdev-docker-entrypoint🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/Dockerfile` at line 137, The Dockerfile uses the underscored opcache_revalidate_freq key which doesn't match the dotted directive used by the entrypoint; update the echo in src/8.5/fpm-alpine/Dockerfile (the line that writes "opcache_revalidate_freq=120") to use the dotted directive name "opcache.revalidate_freq=120" so it matches the logic in src/8.5/fpm-alpine/hiqdev-docker-entrypoint that edits opcache.revalidate_freq.
112-116:⚠️ Potential issue | 🟠 MajorAdd integrity verification before installing downloaded artifacts.
This still installs
gnu-libiconvwith--allow-untrustedover an HTTPlatest-stablerepository and executes a New Relic tarball streamed fromcurlwithout a pinned digest.🛡️ Suggested direction
- && apk add gnu-libiconv=$GNU_LIBICONV_RELEASE --update-cache --repository http://dl-cdn.alpinelinux.org/alpine/latest-stable/community/ --allow-untrusted \ + # Pin the Alpine repository/release and use trusted apk signatures. + && apk add gnu-libiconv=$GNU_LIBICONV_RELEASE --update-cache \ # New Relic - && curl -L https://download.newrelic.com/php_agent/archive/$NEW_RELIC_AGENT_VERSION/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz | tar -C /tmp -zx \ + # Download to a file, verify a pinned SHA256, then extract. + && curl -fsSL -o /tmp/newrelic.tar.gz https://download.newrelic.com/php_agent/archive/$NEW_RELIC_AGENT_VERSION/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz \ + && echo "$NEW_RELIC_AGENT_SHA256 /tmp/newrelic.tar.gz" | sha256sum -c - \ + && tar -C /tmp -zxf /tmp/newrelic.tar.gz \Run this read-only check to confirm no unverified install paths remain:
#!/bin/bash rg -n --context 2 'allow-untrusted|http://dl-cdn|curl .*newrelic|newrelic-install' \ src/8.5/fpm-alpine/Dockerfile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/Dockerfile` around lines 112 - 116, The Dockerfile currently installs gnu-libiconv with --allow-untrusted from an http repo and streams the New Relic tarball directly from curl; fix by switching the apk repo URL to HTTPS and removing --allow-untrusted for the gnu-libiconv install (use the existing GNU_LIBICONV_RELEASE variable and a secure repository), and for the New Relic artifact stop piping curl into tar: download to a file (e.g., /tmp/newrelic.tar.gz using curl -fsSL -o), verify its SHA256 against a pinned NEW_RELIC_AGENT_SHA256 env var (or use a published checksum) with sha256sum -c or equivalent, only then extract and run NR_INSTALL_SILENT=1 /tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl/newrelic-install; also ensure all URLs (the apk repository and New Relic download) use HTTPS and fail the build if checksum verification fails.
🧹 Nitpick comments (6)
src/8.0/fpm-alpine/hiqdev-docker-entrypoint (1)
41-43: Same staleremote_autostartlog line as in 8.1/8.2 — consider updating.The sed fix is correct (adds the missing closing delimiter, previously the command would have errored out as an unterminated
scommand). However, the precedingechostill references the Xdebug 2 name. Matching the phrasing used insrc/8.4/fpm-alpine/hiqdev-docker-entrypointwould keep all entrypoints consistent.✏️ Proposed tweak
if [ -n "$XDEBUG_START_WITH_REQUEST" ]; then - echo "[ xdebug ] Set xdebug.remote_autostart to $XDEBUG_START_WITH_REQUEST" + echo "[ xdebug ] Set xdebug.start_with_request to $XDEBUG_START_WITH_REQUEST" sed -i "s/xdebug.start_with_request\s*=\s*.*/xdebug.start_with_request=$XDEBUG_START_WITH_REQUEST/" /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.0/fpm-alpine/hiqdev-docker-entrypoint` around lines 41 - 43, The log message still uses the old Xdebug 2 name "xdebug.remote_autostart"; update the echo to reflect the new setting by printing something like "[ xdebug ] Set xdebug.start_with_request to $XDEBUG_START_WITH_REQUEST" so it matches the actual change made by the sed on /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini (the XDEBUG_START_WITH_REQUEST branch and the sed command that replaces xdebug.start_with_request).src/8.1/fpm-alpine/hiqdev-docker-entrypoint (1)
61-63: Stale log message: mentionsremote_autostartbut setsxdebug.start_with_request.
xdebug.remote_autostartwas an Xdebug 2 directive; Xdebug 3 replaced it withxdebug.start_with_request. Thesednow correctly targets the new key (with the fixed trailing delimiter — good catch), but theechoon line 62 is still misleading. The same corrected wording was already applied insrc/8.4/fpm-alpine/hiqdev-docker-entrypoint; consider aligning here too.✏️ Proposed tweak
if [ -n "$XDEBUG_START_WITH_REQUEST" ]; then - echo "[ xdebug ] Set xdebug.remote_autostart to $XDEBUG_START_WITH_REQUEST" + echo "[ xdebug ] Set xdebug.start_with_request to $XDEBUG_START_WITH_REQUEST" sed -i "s/xdebug.start_with_request\s*=\s*.*/xdebug.start_with_request=$XDEBUG_START_WITH_REQUEST/" /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.1/fpm-alpine/hiqdev-docker-entrypoint` around lines 61 - 63, The log message incorrectly references the old Xdebug 2 directive "remote_autostart" while the script actually sets xdebug.start_with_request; update the echo on the branch that checks XDEBUG_START_WITH_REQUEST to mention "xdebug.start_with_request" (and optionally mirror the wording used in the 8.4 entrypoint) so the message accurately reflects the variable XDEBUG_START_WITH_REQUEST and the sed change that modifies /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini.src/8.2/fpm-alpine/hiqdev-docker-entrypoint (1)
27-27: Typo and sed terminator fixes LGTM.Note the
sedchange (line 63) fixes a previously malformedscommand that was missing its closing/— same nit about the staleremote_autostartlog line on line 62 as called out insrc/8.1/fpm-alpine/hiqdev-docker-entrypointapplies here; please consider aligning it with the wording already used insrc/8.4/fpm-alpine/hiqdev-docker-entrypoint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.2/fpm-alpine/hiqdev-docker-entrypoint` at line 27, The PR fixed a malformed sed command but left a stale log message; update the log line that mentions "remote_autostart" to match the wording used in the 8.4 entrypoint and verify the sed substitution that disables Opcache uses a proper closing delimiter (the sed 's' command in the Opcache disable block). Concretely, locate the echo/log statement containing "remote_autostart" and replace its text with the exact phrasing from the 8.4 fpm-alpine entrypoint, and ensure the sed command that edits opcache config has a terminating '/' (or consistent delimiter) and is tested to not be missing its closing delimiter.src/7.4/fpm-alpine/hiqdev-docker-entrypoint (1)
10-10: Typo and sed terminator fixes LGTM.Same optional wording suggestion re: the
remote_autostartlog line (line 45) applies here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/7.4/fpm-alpine/hiqdev-docker-entrypoint` at line 10, Update the log/echo message that mentions the remote_autostart flag so its wording is optional/clear: change the current phrasing to something like "remote_autostart is disabled — remote service will not be started automatically" (or similarly explicit) and ensure the message references the remote_autostart variable name; modify the log emission in the hiqdev-docker-entrypoint script where remote_autostart is handled to use this clearer, optional-tone wording.src/8.0/fpm/hiqdev-docker-entrypoint (1)
10-10: Typo and sed terminator fixes LGTM.Same suggestion as on
src/8.0/fpm-alpine/hiqdev-docker-entrypointre: theremote_autostartwording on line 42 — optional cleanup for consistency with 8.4.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.0/fpm/hiqdev-docker-entrypoint` at line 10, Update the wording of the Opcache config reference that currently uses "remote_autostart" to match the 8.4 variant and other script (use "opcache.remote_autostart" in the comment/setting), so the occurrence of remote_autostart around the "Disable Opcache" section is consistent with the fpm-alpine script and 8.4 naming; locate the string "remote_autostart" in the script and replace it with "opcache.remote_autostart" (or adjust the surrounding comment text to explicitly mention "opcache.remote_autostart") to keep terminology uniform.src/8.5/cli-alpine/Dockerfile (1)
1-153: Consider adding a non-root USER directive.The image runs as root by default. While this may be intentional for a CLI image to provide maximum flexibility, running as a non-root user is a security best practice that reduces the attack surface if the container is compromised.
Consider adding a USER directive after line 153:
RUN addgroup -g 1000 appuser && adduser -D -u 1000 -G appuser appuser USER appuserNote: If root access is required for certain operations, you can document this requirement or provide instructions for users to override with
--user rootwhen needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/cli-alpine/Dockerfile` around lines 1 - 153, Add a non-root user after the existing RUN mkdir /app and WORKDIR /app steps: create a group/user (e.g., UID/GID 1000) and chown runtime files/directories (at least /app and any copied binaries like /usr/bin/composer and FreeCDB tools) so the process can drop privileges; then add a USER directive (e.g., USER appuser) to run the container as that user. Locate the Dockerfile symbols RUN mkdir /app, WORKDIR /app, COPY --from=composer and COPY --from=hiqdev/freecdb to ensure you chown the right paths before switching user.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/8.5/cli-alpine/Dockerfile`:
- Around line 45-56: The .build-deps list in the Dockerfile contains a duplicate
entry "zlib-dev"; remove the second occurrence so each dependency (e.g.,
zlib-dev, icu-dev, g++, freetype-dev, libpng-dev, imagemagick-dev,
libjpeg-turbo-dev, ghc, autoconf, krb5-dev, libssh2-dev) appears only once in
the virtual .build-deps package list to keep the dependency list clean.
In `@src/8.5/fpm-alpine/Dockerfile`:
- Around line 94-103: The RUN chain in the Dockerfile is broken by a stray blank
line producing consecutive && operators and opcache is missing; fix by restoring
the opcache install and cleaning up the chain: add a step to install and enable
opcache (e.g., docker-php-ext-install opcache && docker-php-ext-enable opcache)
in the same RUN sequence near the XDebug (pecl install "xdebug-$XDEBUG_RELEASE")
block, and remove the blank line so the logical && chaining between the pecl
installs for rdkafka and memcached and the opcache install is continuous and
valid (refer to symbols XDEBUG_RELEASE, pecl install rdkafka,
docker-php-ext-enable rdkafka, pecl install memcached-$MEMCACHED_RELEASE,
docker-php-ext-enable memcached).
---
Duplicate comments:
In `@src/8.5/fpm-alpine/Dockerfile`:
- Line 137: The Dockerfile uses the underscored opcache_revalidate_freq key
which doesn't match the dotted directive used by the entrypoint; update the echo
in src/8.5/fpm-alpine/Dockerfile (the line that writes
"opcache_revalidate_freq=120") to use the dotted directive name
"opcache.revalidate_freq=120" so it matches the logic in
src/8.5/fpm-alpine/hiqdev-docker-entrypoint that edits opcache.revalidate_freq.
- Around line 112-116: The Dockerfile currently installs gnu-libiconv with
--allow-untrusted from an http repo and streams the New Relic tarball directly
from curl; fix by switching the apk repo URL to HTTPS and removing
--allow-untrusted for the gnu-libiconv install (use the existing
GNU_LIBICONV_RELEASE variable and a secure repository), and for the New Relic
artifact stop piping curl into tar: download to a file (e.g.,
/tmp/newrelic.tar.gz using curl -fsSL -o), verify its SHA256 against a pinned
NEW_RELIC_AGENT_SHA256 env var (or use a published checksum) with sha256sum -c
or equivalent, only then extract and run NR_INSTALL_SILENT=1
/tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl/newrelic-install; also
ensure all URLs (the apk repository and New Relic download) use HTTPS and fail
the build if checksum verification fails.
---
Nitpick comments:
In `@src/7.4/fpm-alpine/hiqdev-docker-entrypoint`:
- Line 10: Update the log/echo message that mentions the remote_autostart flag
so its wording is optional/clear: change the current phrasing to something like
"remote_autostart is disabled — remote service will not be started
automatically" (or similarly explicit) and ensure the message references the
remote_autostart variable name; modify the log emission in the
hiqdev-docker-entrypoint script where remote_autostart is handled to use this
clearer, optional-tone wording.
In `@src/8.0/fpm-alpine/hiqdev-docker-entrypoint`:
- Around line 41-43: The log message still uses the old Xdebug 2 name
"xdebug.remote_autostart"; update the echo to reflect the new setting by
printing something like "[ xdebug ] Set xdebug.start_with_request to
$XDEBUG_START_WITH_REQUEST" so it matches the actual change made by the sed on
/usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini (the
XDEBUG_START_WITH_REQUEST branch and the sed command that replaces
xdebug.start_with_request).
In `@src/8.0/fpm/hiqdev-docker-entrypoint`:
- Line 10: Update the wording of the Opcache config reference that currently
uses "remote_autostart" to match the 8.4 variant and other script (use
"opcache.remote_autostart" in the comment/setting), so the occurrence of
remote_autostart around the "Disable Opcache" section is consistent with the
fpm-alpine script and 8.4 naming; locate the string "remote_autostart" in the
script and replace it with "opcache.remote_autostart" (or adjust the surrounding
comment text to explicitly mention "opcache.remote_autostart") to keep
terminology uniform.
In `@src/8.1/fpm-alpine/hiqdev-docker-entrypoint`:
- Around line 61-63: The log message incorrectly references the old Xdebug 2
directive "remote_autostart" while the script actually sets
xdebug.start_with_request; update the echo on the branch that checks
XDEBUG_START_WITH_REQUEST to mention "xdebug.start_with_request" (and optionally
mirror the wording used in the 8.4 entrypoint) so the message accurately
reflects the variable XDEBUG_START_WITH_REQUEST and the sed change that modifies
/usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini.
In `@src/8.2/fpm-alpine/hiqdev-docker-entrypoint`:
- Line 27: The PR fixed a malformed sed command but left a stale log message;
update the log line that mentions "remote_autostart" to match the wording used
in the 8.4 entrypoint and verify the sed substitution that disables Opcache uses
a proper closing delimiter (the sed 's' command in the Opcache disable block).
Concretely, locate the echo/log statement containing "remote_autostart" and
replace its text with the exact phrasing from the 8.4 fpm-alpine entrypoint, and
ensure the sed command that edits opcache config has a terminating '/' (or
consistent delimiter) and is tested to not be missing its closing delimiter.
In `@src/8.5/cli-alpine/Dockerfile`:
- Around line 1-153: Add a non-root user after the existing RUN mkdir /app and
WORKDIR /app steps: create a group/user (e.g., UID/GID 1000) and chown runtime
files/directories (at least /app and any copied binaries like /usr/bin/composer
and FreeCDB tools) so the process can drop privileges; then add a USER directive
(e.g., USER appuser) to run the container as that user. Locate the Dockerfile
symbols RUN mkdir /app, WORKDIR /app, COPY --from=composer and COPY
--from=hiqdev/freecdb to ensure you chown the right paths before switching user.
🪄 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: b73ca214-4355-4864-a11b-c5dc037d2a3b
📒 Files selected for processing (19)
src/7.0/fpm/Dockerfilesrc/7.0/fpm/hiqdev-docker-entrypointsrc/7.1/fpm/hiqdev-docker-entrypointsrc/7.2/fpm/hiqdev-docker-entrypointsrc/7.3/fpm/hiqdev-docker-entrypointsrc/7.4/fpm-alpine/hiqdev-docker-entrypointsrc/7.4/fpm/hiqdev-docker-entrypointsrc/8.0/fpm-alpine/hiqdev-docker-entrypointsrc/8.0/fpm/hiqdev-docker-entrypointsrc/8.1/fpm-alpine/hiqdev-docker-entrypointsrc/8.2/fpm-alpine/Dockerfilesrc/8.2/fpm-alpine/hiqdev-docker-entrypointsrc/8.3/fpm-alpine/Dockerfilesrc/8.3/fpm-alpine/hiqdev-docker-entrypointsrc/8.4/fpm-alpine/Dockerfilesrc/8.4/fpm-alpine/hiqdev-docker-entrypointsrc/8.5/cli-alpine/Dockerfilesrc/8.5/fpm-alpine/Dockerfilesrc/8.5/fpm-alpine/hiqdev-docker-entrypoint
✅ Files skipped from review due to trivial changes (6)
- src/7.2/fpm/hiqdev-docker-entrypoint
- src/7.4/fpm/hiqdev-docker-entrypoint
- src/7.0/fpm/hiqdev-docker-entrypoint
- src/7.1/fpm/hiqdev-docker-entrypoint
- src/7.3/fpm/hiqdev-docker-entrypoint
- src/8.3/fpm-alpine/hiqdev-docker-entrypoint
🚧 Files skipped from review as they are similar to previous changes (1)
- src/8.5/fpm-alpine/hiqdev-docker-entrypoint
Comment on lines
+45
to
+56
| zlib-dev \ | ||
| icu-dev \ | ||
| g++ \ | ||
| freetype-dev \ | ||
| libpng-dev \ | ||
| imagemagick-dev \ | ||
| libjpeg-turbo-dev \ | ||
| ghc \ | ||
| autoconf \ | ||
| krb5-dev \ | ||
| libssh2-dev \ | ||
| zlib-dev \ |
There was a problem hiding this comment.
Remove duplicate zlib-dev dependency.
zlib-dev appears twice in the .build-deps virtual package list (lines 45 and 56). The duplicate has no effect but clutters the dependency list.
🧹 Proposed fix
icu-dev \
g++ \
freetype-dev \
libpng-dev \
imagemagick-dev \
libjpeg-turbo-dev \
ghc \
autoconf \
krb5-dev \
libssh2-dev \
- zlib-dev \
patch \📝 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
| zlib-dev \ | |
| icu-dev \ | |
| g++ \ | |
| freetype-dev \ | |
| libpng-dev \ | |
| imagemagick-dev \ | |
| libjpeg-turbo-dev \ | |
| ghc \ | |
| autoconf \ | |
| krb5-dev \ | |
| libssh2-dev \ | |
| zlib-dev \ | |
| zlib-dev \ | |
| icu-dev \ | |
| g++ \ | |
| freetype-dev \ | |
| libpng-dev \ | |
| imagemagick-dev \ | |
| libjpeg-turbo-dev \ | |
| ghc \ | |
| autoconf \ | |
| krb5-dev \ | |
| libssh2-dev \ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/cli-alpine/Dockerfile` around lines 45 - 56, The .build-deps list in
the Dockerfile contains a duplicate entry "zlib-dev"; remove the second
occurrence so each dependency (e.g., zlib-dev, icu-dev, g++, freetype-dev,
libpng-dev, imagemagick-dev, libjpeg-turbo-dev, ghc, autoconf, krb5-dev,
libssh2-dev) appears only once in the virtual .build-deps package list to keep
the dependency list clean.
Comment on lines
+94
to
+103
| # XDebug | ||
| pecl install "xdebug-$XDEBUG_RELEASE" && \ | ||
| docker-php-ext-enable xdebug && \ | ||
|
|
||
| && pecl install rdkafka \ | ||
| && docker-php-ext-enable rdkafka \ | ||
|
|
||
| # Memcached | ||
| && pecl install memcached-$MEMCACHED_RELEASE \ | ||
| && docker-php-ext-enable memcached \ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
path = Path("src/8.5/fpm-alpine/Dockerfile")
text = path.read_text()
logical = text.replace("\\\n", " ")
needle = "docker-php-ext-enable xdebug && && pecl install rdkafka"
print("broken_chain_found=", needle in logical)
print("opcache_install_found=", "docker-php-ext-install opcache" in text)
PYRepository: hiqdev/docker-php
Length of output: 115
🏁 Script executed:
sed -n '94,103p' src/8.5/fpm-alpine/DockerfileRepository: hiqdev/docker-php
Length of output: 337
🏁 Script executed:
#!/bin/bash
# Check the logical line to see actual spacing
python - <<'PY'
from pathlib import Path
path = Path("src/8.5/fpm-alpine/Dockerfile")
text = path.read_text()
logical = text.replace("\\\n", " ")
# Find the xdebug to rdkafka portion
import re
match = re.search(r'docker-php-ext-enable xdebug.*?pecl install rdkafka', logical, re.DOTALL)
if match:
segment = match.group(0)
# Show the exact characters including spaces
print("Actual logical text segment:")
print(repr(segment[:80]))
print("\nPattern has && followed by &&:", "&&" in segment and segment.count("&&") >= 2)
PYRepository: hiqdev/docker-php
Length of output: 191
Restore the missing opcache install to fix the broken RUN chain.
Line 96 ends with && \, then line 98 starts with another &&, creating consecutive && operators with only a blank line between them. This produces an invalid shell command and breaks the Docker build. Additionally, opcache is not installed despite later configuration referencing it.
Proposed fix
# XDebug
pecl install "xdebug-$XDEBUG_RELEASE" && \
docker-php-ext-enable xdebug && \
+ # Opcache
+ docker-php-ext-install opcache && \
+
&& pecl install rdkafka \
&& docker-php-ext-enable rdkafka \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/8.5/fpm-alpine/Dockerfile` around lines 94 - 103, The RUN chain in the
Dockerfile is broken by a stray blank line producing consecutive && operators
and opcache is missing; fix by restoring the opcache install and cleaning up the
chain: add a step to install and enable opcache (e.g., docker-php-ext-install
opcache && docker-php-ext-enable opcache) in the same RUN sequence near the
XDebug (pecl install "xdebug-$XDEBUG_RELEASE") block, and remove the blank line
so the logical && chaining between the pecl installs for rdkafka and memcached
and the opcache install is continuous and valid (refer to symbols
XDEBUG_RELEASE, pecl install rdkafka, docker-php-ext-enable rdkafka, pecl
install memcached-$MEMCACHED_RELEASE, docker-php-ext-enable memcached).
There was a problem hiding this comment.
♻️ Duplicate comments (6)
src/8.5/fpm-alpine/Dockerfile (3)
115-117:⚠️ Potential issue | 🟠 MajorAdd integrity verification for New Relic agent archive before execution.
Lines 115-117 download/extract/execute as root without checksum validation.
#!/bin/bash # Verify there is no checksum/signature validation around New Relic install rg -n 'newrelic-php5-|newrelic-install|sha256|sha512|gpg' src/8.5/fpm-alpine/DockerfileSuggested diff
+ENV NEW_RELIC_AGENT_SHA256=<pin-the-official-sha256> ... - && curl -L https://download.newrelic.com/php_agent/archive/$NEW_RELIC_AGENT_VERSION/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz | tar -C /tmp -zx \ + && curl -fsSL -o /tmp/newrelic.tar.gz "https://download.newrelic.com/php_agent/archive/$NEW_RELIC_AGENT_VERSION/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz" \ + && echo "$NEW_RELIC_AGENT_SHA256 /tmp/newrelic.tar.gz" | sha256sum -c - \ + && tar -C /tmp -xzf /tmp/newrelic.tar.gz \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/Dockerfile` around lines 115 - 117, The Dockerfile currently pipes the New Relic agent archive from curl directly into tar and runs newrelic-install without integrity checks; change the sequence around the NEW_RELIC_AGENT_VERSION/newrelic-php5 download to first fetch both the .tar.gz and its canonical checksum (e.g., .sha256 or .sha512) using curl -fSL into /tmp, verify the archive with sha256sum (or sha512sum) -c and abort on mismatch, only then extract the verified /tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz and run NR_INSTALL_* /tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl/newrelic-install, and finally rm -rf the /tmp/newrelic-php5-* and checksum files to keep the existing cleanup semantics.
112-112:⚠️ Potential issue | 🟠 MajorAvoid
--allow-untrustedwithlatest-stablerepo forgnu-libiconv.Line 112 weakens package signature guarantees and hurts reproducibility. Pin to a versioned Alpine repo and install with normal trust chain.
#!/bin/bash # Verify risky apk usage in 8.5 Dockerfiles rg -n --iglob '*Dockerfile' 'gnu-libiconv|--allow-untrusted|alpine/latest-stable/community'Suggested diff
- && apk add gnu-libiconv=$GNU_LIBICONV_RELEASE --update-cache --repository http://dl-cdn.alpinelinux.org/alpine/latest-stable/community/ --allow-untrusted \ + && apk add --no-cache gnu-libiconv=$GNU_LIBICONV_RELEASE --repository http://dl-cdn.alpinelinux.org/alpine/v3.21/community/ \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/Dockerfile` at line 112, The Dockerfile uses apk add with --allow-untrusted and the latest-stable repo for gnu-libiconv (the line installing gnu-libiconv via GNU_LIBICONV_RELEASE), which weakens signature guarantees; change the repository to a specific Alpine release (e.g., use an ALPINE_VERSION variable and point to http://dl-cdn.alpinelinux.org/alpine/v${ALPINE_VERSION}/community/) and remove --allow-untrusted, and install with standard apk flags (e.g., apk add --no-cache gnu-libiconv=${GNU_LIBICONV_RELEASE}) so the package is fetched from a pinned, signed repo and signature verification is preserved.
137-137:⚠️ Potential issue | 🔴 CriticalFix invalid ini key: use
opcache.revalidate_freq(dot), not underscore.Line 137 currently writes a non-standard key and won’t match the entrypoint replacement pattern.
#!/bin/bash # Verify directive mismatch between Dockerfile and entrypoint substitution rg -n 'opcache_revalidate_freq|opcache\.revalidate_freq' \ src/8.5/fpm-alpine/Dockerfile \ src/8.5/cli-alpine/Dockerfile \ src/8.5/fpm-alpine/hiqdev-docker-entrypointSuggested diff
- echo "opcache_revalidate_freq=120"; \ + echo "opcache.revalidate_freq=120"; \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/Dockerfile` at line 137, The Dockerfile writes an invalid INI key using opcache_revalidate_freq (underscore); update that echo line to use the correct directive opcache.revalidate_freq (dot) so it matches the entrypoint substitution logic used in hiqdev-docker-entrypoint; locate the echo that emits "opcache_revalidate_freq=120" and change the key to "opcache.revalidate_freq=120".src/8.5/cli-alpine/Dockerfile (3)
101-101:⚠️ Potential issue | 🟠 MajorReplace untrusted
gnu-libiconvinstall path with pinned trusted repo usage.Line 101 uses
latest-stableplus--allow-untrusted, which is a supply-chain and reproducibility risk.#!/bin/bash # Verify risky package install flags and repo source rg -n --iglob '*Dockerfile' 'gnu-libiconv|--allow-untrusted|alpine/latest-stable/community'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/cli-alpine/Dockerfile` at line 101, The Dockerfile line installing gnu-libiconv uses the unstable "latest-stable" repo and --allow-untrusted; change that apk add invocation to use a pinned, trusted Alpine repo and drop --allow-untrusted. Specifically, replace the repository URL fragment "alpine/latest-stable/community" with a pinned release variable (e.g., use an existing ALPINE_VERSION or introduce one) and remove the --allow-untrusted flag in the apk add that installs gnu-libiconv (the line referencing gnu-libiconv and GNU_LIBICONV_RELEASE); ensure you use the official v$ALPINE_VERSION/community mirror (or a configured secure mirror) and keep --no-cache or --update-cache as appropriate to preserve reproducibility and supply-chain trust.
45-56:⚠️ Potential issue | 🟡 MinorDeduplicate
zlib-devin.build-deps.Line 45 and Line 56 both include
zlib-dev; keep one entry.Suggested diff
krb5-dev \ libssh2-dev \ - zlib-dev \ patch \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/cli-alpine/Dockerfile` around lines 45 - 56, The .build-deps package list in the Dockerfile contains a duplicated entry "zlib-dev" (appearing twice in the shown package block); remove the redundant "zlib-dev" so it appears only once in the .build-deps/package installation list to avoid duplication and keep the list clean.
121-121:⚠️ Potential issue | 🔴 CriticalUse
opcache.revalidate_freqinstead ofopcache_revalidate_freq.Line 121 has the same invalid-key typo; PHP ini should use dotted directive names.
#!/bin/bash # Verify all opcache revalidate directive forms in PHP 8.5 Dockerfiles/entrypoint rg -n 'opcache_revalidate_freq|opcache\.revalidate_freq' \ src/8.5/cli-alpine/Dockerfile \ src/8.5/fpm-alpine/Dockerfile \ src/8.5/fpm-alpine/hiqdev-docker-entrypointSuggested diff
- echo "opcache_revalidate_freq=120"; \ + echo "opcache.revalidate_freq=120"; \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/cli-alpine/Dockerfile` at line 121, The echoed PHP ini directive uses the wrong underscore form; locate the echo that outputs "opcache_revalidate_freq=120" in the Dockerfile and change the directive name to the dotted form "opcache.revalidate_freq=120"; also search for any other occurrences of "opcache_revalidate_freq" in this Dockerfile (and related entrypoint files) and replace them with "opcache.revalidate_freq" to ensure PHP reads the setting correctly.
🧹 Nitpick comments (1)
src/8.5/fpm-alpine/Dockerfile (1)
56-67: Remove duplicatezlib-devfrom.build-deps.
zlib-devis listed twice (Line 56 and Line 67). Keep one.Suggested diff
krb5-dev \ libssh2-dev \ - zlib-dev \ patch \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/8.5/fpm-alpine/Dockerfile` around lines 56 - 67, The .build-deps package list contains a duplicate entry for zlib-dev; edit the Dockerfile's .build-deps list (the multi-line dependency block that includes zlib-dev, icu-dev, g++, etc.) and remove the second occurrence of zlib-dev so it only appears once in the dependency list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/8.5/cli-alpine/Dockerfile`:
- Line 101: The Dockerfile line installing gnu-libiconv uses the unstable
"latest-stable" repo and --allow-untrusted; change that apk add invocation to
use a pinned, trusted Alpine repo and drop --allow-untrusted. Specifically,
replace the repository URL fragment "alpine/latest-stable/community" with a
pinned release variable (e.g., use an existing ALPINE_VERSION or introduce one)
and remove the --allow-untrusted flag in the apk add that installs gnu-libiconv
(the line referencing gnu-libiconv and GNU_LIBICONV_RELEASE); ensure you use the
official v$ALPINE_VERSION/community mirror (or a configured secure mirror) and
keep --no-cache or --update-cache as appropriate to preserve reproducibility and
supply-chain trust.
- Around line 45-56: The .build-deps package list in the Dockerfile contains a
duplicated entry "zlib-dev" (appearing twice in the shown package block); remove
the redundant "zlib-dev" so it appears only once in the .build-deps/package
installation list to avoid duplication and keep the list clean.
- Line 121: The echoed PHP ini directive uses the wrong underscore form; locate
the echo that outputs "opcache_revalidate_freq=120" in the Dockerfile and change
the directive name to the dotted form "opcache.revalidate_freq=120"; also search
for any other occurrences of "opcache_revalidate_freq" in this Dockerfile (and
related entrypoint files) and replace them with "opcache.revalidate_freq" to
ensure PHP reads the setting correctly.
In `@src/8.5/fpm-alpine/Dockerfile`:
- Around line 115-117: The Dockerfile currently pipes the New Relic agent
archive from curl directly into tar and runs newrelic-install without integrity
checks; change the sequence around the NEW_RELIC_AGENT_VERSION/newrelic-php5
download to first fetch both the .tar.gz and its canonical checksum (e.g.,
.sha256 or .sha512) using curl -fSL into /tmp, verify the archive with sha256sum
(or sha512sum) -c and abort on mismatch, only then extract the verified
/tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl.tar.gz and run
NR_INSTALL_*
/tmp/newrelic-php5-$NEW_RELIC_AGENT_VERSION-linux-musl/newrelic-install, and
finally rm -rf the /tmp/newrelic-php5-* and checksum files to keep the existing
cleanup semantics.
- Line 112: The Dockerfile uses apk add with --allow-untrusted and the
latest-stable repo for gnu-libiconv (the line installing gnu-libiconv via
GNU_LIBICONV_RELEASE), which weakens signature guarantees; change the repository
to a specific Alpine release (e.g., use an ALPINE_VERSION variable and point to
http://dl-cdn.alpinelinux.org/alpine/v${ALPINE_VERSION}/community/) and remove
--allow-untrusted, and install with standard apk flags (e.g., apk add --no-cache
gnu-libiconv=${GNU_LIBICONV_RELEASE}) so the package is fetched from a pinned,
signed repo and signature verification is preserved.
- Line 137: The Dockerfile writes an invalid INI key using
opcache_revalidate_freq (underscore); update that echo line to use the correct
directive opcache.revalidate_freq (dot) so it matches the entrypoint
substitution logic used in hiqdev-docker-entrypoint; locate the echo that emits
"opcache_revalidate_freq=120" and change the key to
"opcache.revalidate_freq=120".
---
Nitpick comments:
In `@src/8.5/fpm-alpine/Dockerfile`:
- Around line 56-67: The .build-deps package list contains a duplicate entry for
zlib-dev; edit the Dockerfile's .build-deps list (the multi-line dependency
block that includes zlib-dev, icu-dev, g++, etc.) and remove the second
occurrence of zlib-dev so it only appears once in the dependency list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b5674b7-b9db-466e-8607-a2ee3d636b5f
📒 Files selected for processing (2)
src/8.5/cli-alpine/Dockerfilesrc/8.5/fpm-alpine/Dockerfile
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 by CodeRabbit