Add Alpine musl support using custom Go toolchain - #466
Conversation
| RUN git clone --depth 1 --branch "${PHP_SRC_REF}" \ | ||
| https://github.com/php/php-src.git |
There was a problem hiding this comment.
🟡 Medium - Pin all upstream inputs used to produce release Alpine binaries
The Alpine release path clones PHP-${PHP_VERSION} by branch name and builds on mutable alpine:3.16/alpine:3.22 images with unversioned apk dependencies, so the bytes that enter a tagged release can change without any repository change. A compromised or replaced upstream branch, image, or package repository can therefore inject code into the PHP extension or package that this workflow publishes. This is inconsistent with the digest, archive-hash, and exact-commit verification already used for the Go and Zen inputs.
Show fix
Pin each base image by digest, fetch PHP from an exact reviewed commit and verify it, and lock Alpine repositories/packages (or otherwise record and verify immutable package versions) for the release build and validation images.
More info - Reply on this comment to give feedback or ignore the issue.
There was a problem hiding this comment.
Valid but should do this afterwards for all images, not just alpine. Ignore
There was a problem hiding this comment.
🔴 We were not able to ignore this issue because of the following reason:
You do not have the permission to ignore issues.
| RUN abuild-keygen -a -n | ||
|
|
||
| USER root | ||
| RUN cp /home/builder/.abuild/*.rsa.pub /etc/apk/keys/ |
There was a problem hiding this comment.
🟡 Medium - Publish a stable APK verification key with the Alpine release
Each APK is signed with a key generated inside the ephemeral package-build container, but the public key is not published alongside the APK and both CI checks install it with --allow-untrusted. Consequently consumers have no repository-controlled trust anchor and must bypass APK signature trust; if the release artifact is replaced, its root-run installation scripts can be executed without a verifiable publisher signature. The in-image verification only proves that the package matches a key that disappears with the build container.
Show fix
Sign release APKs with a persistent publisher key, publish the corresponding public key (or a signed APK repository/index and digest manifest), and make verification and installation fail for packages that are not trusted rather than using --allow-untrusted.
More info - Reply on this comment to give feedback or ignore the issue.
There was a problem hiding this comment.
Again valid for all images not just alpine, ignore
There was a problem hiding this comment.
🔴 We were not able to ignore this issue because of the following reason:
You do not have the permission to ignore issues.
| license="AGPL-3.0-or-later" | ||
| # The runtime currently resolves its versioned binaries from /opt/aikido-*. | ||
| options="!check !strip !fhs" | ||
| install="$pkgname.post-install $pkgname.post-upgrade $pkgname.pre-deinstall" |
There was a problem hiding this comment.
🔵 Low - Make the APKBUILD self-contained by adding the declared post-upgrade script
The APKBUILD declares aikido-php-firewall.post-upgrade as an install script, but that file is not present under package/apk; only the workflow creates it immediately before running abuild. A normal maintainer or downstream build invoking this APKBUILD directly therefore cannot resolve the declared install script and fails or produces an incomplete package unless it knows about the CI-only preprocessing step. The packaging definition is consequently not independently buildable as committed.
Show fix
Commit the post-upgrade script (or remove it from the APKBUILD and use a supported Alpine upgrade-script mechanism), so abuild can build the package directly without workflow-specific file generation.
More info - Reply on this comment to give feedback or ignore the issue.
There was a problem hiding this comment.
The resulting APK contains .post-upgrade derived from post-install, and its presence and behavior are verified during the package build and lifecycle tests. Ignore
There was a problem hiding this comment.
🔴 We were not able to ignore this issue because of the following reason:
You do not have the permission to ignore issues.
| for php_bin in $PHP_BINS; do | ||
| extension_dir=$("$php_bin" -r 'echo ini_get("extension_dir");' 2>/dev/null) || continue | ||
| scan_dir=$("$php_bin" --ini 2>/dev/null | awk -F ': ' '/Scan for additional .ini files in/ { print $2; exit }') | ||
| rm -f "$extension_dir/aikido-${VERSION}.so" | ||
| case "$scan_dir" in | ||
| ""|"(none)") ;; | ||
| *) rm -f "$scan_dir/zz-aikido-${VERSION}.ini" ;; | ||
| esac |
There was a problem hiding this comment.
🔵 Low - Remove PHP extension links even when PHP is absent during APK removal
The deinstallation script only computes extension and INI paths by running PHP binaries that still exist, so removing PHP first leaves the package-created aikido-${VERSION}.so links and zz-aikido-${VERSION}.ini links behind. Those dangling files are not owned by the APK payload and are therefore not removed later when the package is deleted; reinstalling PHP can also encounter stale Aikido configuration. This makes uninstall order affect system cleanup and leaves the firewall installation partially present after removal.
Show fix
Record the target PHP extension and scan directories when installing, or enumerate the package's versioned links/configuration paths independently of currently installed PHP binaries so deinstallation can remove them even after PHP has been removed.
More info - Reply on this comment to give feedback or ignore the issue.
There was a problem hiding this comment.
The current RPM and converted DEB lifecycle scripts also discover cleanup paths through installed PHP binaries. The Alpine package follows that existing behavior, and the supported uninstall path with PHP present is covered by CI. Supporting cleanup after PHP itself has already been removed should be addressed consistently across all package formats in a separate lifecycle change.
Summary
Custom Go toolchain
The Alpine build temporarily uses a custom Go source commit containing the runtime and linker changes needed to load C-shared Go libraries on musl. The Alpine build first uses Go 1.26.7 to compile the pinned custom Go toolchain, then uses that custom toolchain for all Alpine Go tests and artifacts. Tests and shipped Go artifacts use external linking to preserve the optional libc-detection symbol correctly on musl.
Validation
dlopensmoke testThe full x86_64/aarch64 PHP 7.2-8.5 NTS/ZTS matrix and APK install/remove checks pass in CI.