Conversation
Bumps [aquasecurity/trivy-action](https://github.com/aquasecurity/trivy-action) from 0.33.1 to 0.34.0. - [Release notes](https://github.com/aquasecurity/trivy-action/releases) - [Commits](aquasecurity/trivy-action@0.33.1...0.34.0) --- updated-dependencies: - dependency-name: aquasecurity/trivy-action dependency-version: 0.34.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
…hub/workflows/aquasecurity/trivy-action-0.34.0 Bump aquasecurity/trivy-action from 0.33.1 to 0.34.0 in /.github/workflows
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR synchronizes multiple shell scripts and GitHub workflows. The primary changes include adding CloudFlare DNS fallback logic to Let's Encrypt certificate management scripts, standardizing /run/php directory permissions to 0777 across multiple scripts, updating the Trivy vulnerability scanner action version, and adding a new AARCH64 container build workflow.
Changes:
- Added CloudFlare DNS fallback mechanism in Let's Encrypt certificate creation and renewal scripts to retry with webroot method if CloudFlare API fails
- Standardized /run/php permissions to 0777 in both reloadPHPfpm.sh and applypermissions.sh
- Updated Trivy action from 0.33.1 to 0.34.0 and added new AARCH64 workflow file
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| core_files/intcmd/reloadPHPfpm.sh | Added explicit chmod/chown commands for /run/php before reloading PHP-FPM services |
| core_files/intcmd/letsencrypt/renewLEAllCert.sh | Refactored to loop through certificates and added CloudFlare fallback logic with webroot retry |
| core_files/intcmd/letsencrypt/createLECert.sh | Added CloudFlare DNS failure detection and automatic fallback to webroot certificate method |
| core_files/intcmd/applypermissions.sh | Added special case handling to set /run/php permissions to 0777 instead of default 0755 |
| .github/workflows/build_container_template.yml | Updated aquasecurity/trivy-action from version 0.33.1 to 0.34.0 |
| .github/disabled_workflows/build_containers_aarch64.yml | Added new workflow for building containers on AARCH64 architecture using Raspberry Pi 5 runner |
Comments suppressed due to low confidence (4)
core_files/intcmd/reloadPHPfpm.sh:25
- The order of operations (chmod before chown) could be problematic. When chmod is applied before chown, the permissions are set while the directory may still be owned by root, and then ownership is transferred. This is generally acceptable, but the more common and safer practice is to set ownership first (chown) and then permissions (chmod), as seen in other scripts in the codebase like core_files/intcmd/background/background_le_ssl_renew.sh. Consider swapping the order for consistency.
chmod 0777 -R /run/php
chown www-data:www-data -R /run/php
core_files/intcmd/letsencrypt/createLECert.sh:24
- The unset operation on line 24 is unnecessary and appears to be a logic error. The variable CLOUDFLARE_USED is already set to false on line 20, so unsetting it before setting it to true serves no purpose. This line should be removed.
unset CLOUDFLARE_USED
core_files/intcmd/letsencrypt/renewLEAllCert.sh:47
- These lines have inconsistent indentation. Lines 46-47 appear to have an extra leading space compared to the surrounding code. The indentation should match the other lines within the same if block (lines 44-45 and 48-54).
WEBROOT_OPTS="--webroot --webroot-path $certStuffRoot"
certbot renew --config-dir $sslDirPath $WEBROOT_OPTS --cert-name "$certName"
core_files/intcmd/letsencrypt/createLECert.sh:47
- There's a spelling error: "you're" should be "your". The correct phrase is "Enjoy your new cert".
echo "End of script have a nice day! Enjoy you're new cert if it was created"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| certbot certonly --config-dir $sslDirPath $WEBROOT_OPTS -n --agree-tos --register-unsafely-without-email -d ${1} | ||
| if [ $? -ne 0 ]; then | ||
| echo "[Failure] Unable to create certificate '${1}' due to an error" | ||
| if [ $CLOUDFLARE_USED = true ]; then |
There was a problem hiding this comment.
The variable comparison is missing quotes around the variable. This should be if [ "$CLOUDFLARE_USED" = true ]; then for consistency with the same comparison in renewLEAllCert.sh line 43 and to prevent potential issues if the variable is empty or contains spaces.
| if [ $CLOUDFLARE_USED = true ]; then | |
| if [ "$CLOUDFLARE_USED" = true ]; then |
| function renew() { | ||
| local certName="$1" | ||
| local WEBROOT_OPTS="--webroot --webroot-path $certStuffRoot" | ||
| local CLOUDFLARE_USED=false | ||
| if [ -f "/cloudflare-account.ini" ]; then | ||
| echo "Using CloudFlare API for DNS" | ||
| WEBROOT_OPTS="--dns-cloudflare --dns-cloudflare-credentials /cloudflare-account.ini" | ||
| CLOUDFLARE_USED=true | ||
| fi | ||
| echo "Renewing certificate for $certName" | ||
| certbot renew --config-dir $sslDirPath $WEBROOT_OPTS --cert-name "$certName" | ||
| if [ $? -ne 0 ]; then | ||
| echo "Failed to renew certificate for $certName" | ||
| if [ "$CLOUDFLARE_USED" = true ]; then | ||
| echo "Please check your CloudFlare API credentials and permissions. Using webroot method as a fallback." | ||
| unset WEBROOT_OPTS | ||
| WEBROOT_OPTS="--webroot --webroot-path $certStuffRoot" | ||
| certbot renew --config-dir $sslDirPath $WEBROOT_OPTS --cert-name "$certName" | ||
| if [ $? -ne 0 ]; then | ||
| echo "Failed to renew certificate for $certName using webroot method as well." | ||
| return 1 | ||
| else | ||
| echo "Successfully renewed certificate for $certName using webroot method." | ||
| return 0 | ||
| fi | ||
| fi | ||
| return 1 | ||
| fi | ||
| } |
There was a problem hiding this comment.
The refactoring removed the logging of certbot output to the log file. Previously, certbot output was redirected to the log file with >> /scripts/letsencrypt/letsencrypt-renew.log. Now the renew function outputs directly to stdout/stderr without any file logging. The logFile variable is created but never used. Either remove the unused logFile variable (lines 13, 25-28) or restore logging functionality by redirecting certbot output to the log file.
| for certPath in "$sslLiveDirPath"/*; do | ||
| if [ -d "$certPath" ]; then | ||
| certName=$(basename "$certPath") | ||
| renew "$certName" | ||
| fi | ||
| done |
There was a problem hiding this comment.
The loop does not handle the case where the sslLiveDirPath does not exist or contains no subdirectories. If the directory doesn't exist or is empty, the glob pattern will fail silently without any feedback to the user. Consider adding a check before the loop to ensure the directory exists and contains certificates to renew, similar to how other directories are validated earlier in the script.
| echo "$service_name started successfully." | ||
| } | ||
|
|
||
| chmod 0777 -R /run/php |
There was a problem hiding this comment.
The recursive chmod 0777 on /run/php combined with chown www-data:www-data makes the PHP-FPM runtime directory and its Unix sockets world-readable and world-writable. Any local user or compromised process on the host can then connect directly to the PHP-FPM socket, bypassing web server access controls (auth, IP restrictions, TLS) and interact with PHP applications as www-data, which can lead to privilege escalation and unauthorized data access. Restrict the directory and socket permissions to the minimum required (e.g., 0750/0660 with a dedicated group) instead of 0777.
| chmod 0777 -R /run/php | |
| chmod 0750 -R /run/php |
| FINAL_PERMCODE=0777 | ||
| echo "[INFO] Special case for $path: setting permissions to $FINAL_PERMCODE" |
There was a problem hiding this comment.
Special-casing /run/php to always use 0777 recursively for FINAL_PERMCODE makes the PHP-FPM runtime directory and its contents world-writable. This allows any local user or compromised process on the host to connect to or tamper with PHP-FPM Unix sockets, bypassing web server authentication and IP/TLS protections and potentially gaining unauthorized access to PHP applications or data. Use a restrictive permission mask (e.g., 0750 on the directory and 0660 on sockets with a dedicated group) instead of 0777 for /run/php.
| FINAL_PERMCODE=0777 | |
| echo "[INFO] Special case for $path: setting permissions to $FINAL_PERMCODE" | |
| FINAL_PERMCODE=0750 | |
| echo "[INFO] Special case for $path: setting permissions to $FINAL_PERMCODE (restricted for PHP-FPM runtime)" |
No description provided.