Skip to content

fix: harden sed expressions and add missing file guard in php-update.sh - #234

Merged
PDowney merged 4 commits into
masterfrom
copilot/fix-socket-and-fastcgi-expressions
May 2, 2026
Merged

PDowney merged 4 commits into
masterfrom
copilot/fix-socket-and-fastcgi-expressions

Conversation

Copilot AI commented May 2, 2026

Copy link
Copy Markdown
Contributor
  • Fix sed escaping, syntax, and missing file guard in php-update.sh (previous session)
  • Add local variable for positional parameter and explicit return in sed_escape_ere()
  • Add local variable for positional parameter and explicit return in sed_escape_replacement()
  • Update CHANGELOG.md
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The printf format strings SOCKET_EXPR and FASTCGI_EXPR use %s placeholders but receive unquoted variables. While this works, it's fragile if version strings contain special characters. Consider using sed's standard delimiter replacement or properly quoting the variables in the printf calls.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -135,12 +135,24 @@\n SOCKET_EXPR='s|(unix:/run/php/)php%s-fpm(\\.sock)|\\1php%s-fpm\\2|g'\n FASTCGI_EXPR='s|(fastcgi_pass[[:space:]]+[^;]*php)%s(-fpm)|\\1%s\\2|g'\n \n+# Escape text for use in sed extended regex pattern fragments.\n+sed_escape_ere() {\n+    printf '%s' \"$1\" | sed -e 's/[][(){}.^$*+?|\\\\]/\\\\&/g'\n+}\n+\n+# Escape text for use in sed replacement fragments.\n+sed_escape_replacement() {\n+    printf '%s' \"$1\" | sed -e 's/[&\\\\]/\\\\&/g'\n+}\n+\n # Update php-fpm.conf\n if [[ -f \"/etc/nginx/globals/php-fpm.conf\" ]]; then\n     for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n+        OLD_VER_ERE=\"$(sed_escape_ere \"$OLD_VER\")\"\n+        NEW_PHP_VER_REPL=\"$(sed_escape_replacement \"$NEW_PHP_VER\")\"\n         sed -E -i \\\n-            -e \"$(printf \"$SOCKET_EXPR\" \"$OLD_VER\" \"$NEW_PHP_VER\")\" \\\n-            -e \"$(printf \"$FASTCGI_EXPR\" \"$OLD_VER\" \"$NEW_PHP_VER\")\" \\\n+            -e \"$(printf \"$SOCKET_EXPR\" \"$OLD_VER_ERE\" \"$NEW_PHP_VER_REPL\")\" \\\n+            -e \"$(printf \"$FASTCGI_EXPR\" \"$OLD_VER_ERE\" \"$NEW_PHP_VER_REPL\")\" \\\n             \"/etc/nginx/globals/php-fpm.conf\"\n     done\n fi\n@@ -149,7 +157,9 @@\n for config_file in /etc/nginx/sites-available/*; do\n     if [[ -f \"$config_file\" ]]; then\n         for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-            sed -E -i -e \"/^[[:space:]]*fastcgi_pass[[:space:]]+/ $(printf \"$FASTCGI_EXPR\" \"$OLD_VER\" \"$NEW_PHP_VER\")\" \"$config_file\"\n+            OLD_VER_ERE=\"$(sed_escape_ere \"$OLD_VER\")\"\n+            NEW_PHP_VER_REPL=\"$(sed_escape_replacement \"$NEW_PHP_VER\")\"\n+            sed -E -i -e \"/^[[:space:]]*fastcgi_pass[[:space:]]+/ $(printf \"$FASTCGI_EXPR\" \"$OLD_VER_ERE\" \"$NEW_PHP_VER_REPL\")\" \"$config_file\"\n         done\n     fi\n done\n"}]},{"message":"The sed command has incorrect syntax. The `-e` flag is followed by a space before the pattern, which causes the command substitution result to be treated as a separate argument rather than part of the expression. Remove the space after `-e` or the `-e` flag entirely since only one expression is being used.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -149,7 +149,7 @@\n for config_file in /etc/nginx/sites-available/*; do\n     if [[ -f \"$config_file\" ]]; then\n         for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-            sed -E -i -e \"/^[[:space:]]*fastcgi_pass[[:space:]]+/ $(printf \"$FASTCGI_EXPR\" \"$OLD_VER\" \"$NEW_PHP_VER\")\" \"$config_file\"\n+            sed -E -i -e \"/^[[:space:]]*fastcgi_pass[[:space:]]+/$(printf \"$FASTCGI_EXPR\" \"$OLD_VER\" \"$NEW_PHP_VER\")\" \"$config_file\"\n         done\n     fi\n done\n"}]},{"message":"The install-state.conf file update assumes the file exists when appending in the else branch. If the file or directory doesn't exist, the echo command will fail. Add a check to create the directory and file if they don't exist before attempting to write.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -185,6 +185,8 @@\n STATUS=\"$(systemctl is-active \"php${NEW_PHP_VER}-fpm\")\"\n if [[ \"${STATUS}\" == \"active\" ]]; then\n     echo \"PASSED: PHP ${NEW_PHP_VER} is running.\"\n+    mkdir -p /etc/enginescript\n+    touch /etc/enginescript/install-state.conf\n     if grep -q '^PHP=' /etc/enginescript/install-state.conf; then\n         sed -i 's/^PHP=.*/PHP=1/' /etc/enginescript/install-state.conf\n     else\n"}]}]

Copilot AI changed the title [WIP] Fix printf format strings in php-update.sh for safety fix: harden sed expressions and add missing file guard in php-update.sh May 2, 2026
Copilot AI requested a review from PDowney May 2, 2026 07:47
Copilot finished work on behalf of PDowney May 2, 2026 07:47
Copilot finished work on behalf of PDowney May 2, 2026 08:09
Removed the entry for the PHP update script robustness improvements from the changelog.
@github-actions

github-actions Bot commented May 2, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@PDowney
PDowney marked this pull request as ready for review May 2, 2026 08:13
Copilot AI review requested due to automatic review settings May 2, 2026 08:13
@PDowney
PDowney merged commit 90d117a into master May 2, 2026
5 of 6 checks passed
@sonarqubecloud

sonarqubecloud Bot commented May 2, 2026

Copy link
Copy Markdown

@github-actions
github-actions Bot deleted the copilot/fix-socket-and-fastcgi-expressions branch May 2, 2026 08:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the PHP upgrade flow by making the Nginx config migrations in php-update.sh more resilient (safer sed expression construction) and by ensuring the install-state file exists before it’s updated.

Changes:

  • Added sed_escape_ere() and sed_escape_replacement() helpers and used them when generating version-migration sed commands.
  • Fixed the sed -e invocation used for updating sites-available configs and added a guard to create /etc/enginescript/install-state.conf before editing it.
  • Updated CHANGELOG.md.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

File Description
scripts/update/php-update.sh Escapes version strings used in sed patterns/replacements and adds a missing file/dir guard for install-state tracking.
CHANGELOG.md Documents the change.

Comment on lines 135 to 136
SOCKET_EXPR='s|(unix:/run/php/)php%s-fpm(\.sock)|\1php%s-fpm\2|g'
FASTCGI_EXPR='s|(fastcgi_pass[[:space:]]+[^;]*php)%s(-fpm)|\1%s\2|g'
# Escape text for use in sed replacement fragments.
sed_escape_replacement() {
local text="$1"
printf '%s' "$text" | sed -e 's/[&\\]/\\&/g'
Comment on lines +155 to +156
OLD_VER_ERE="$(sed_escape_ere "$OLD_VER")"
NEW_PHP_VER_REPL="$(sed_escape_replacement "$NEW_PHP_VER")"
Comment on lines +168 to +169
OLD_VER_ERE="$(sed_escape_ere "$OLD_VER")"
NEW_PHP_VER_REPL="$(sed_escape_replacement "$NEW_PHP_VER")"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants