fix: harden sed expressions and add missing file guard in php-update.sh - #234
Merged
Merged
Conversation
Agent-Logs-Url: https://github.com/EngineScript/EngineScript/sessions/789782f4-8404-498f-a772-b7901f66fb69 Co-authored-by: PDowney <11467177+PDowney@users.noreply.github.com>
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
…tions Agent-Logs-Url: https://github.com/EngineScript/EngineScript/sessions/eed53274-aa12-4000-b441-b4076d915e9e Co-authored-by: PDowney <11467177+PDowney@users.noreply.github.com>
Removed the entry for the PHP update script robustness improvements from the changelog.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
PDowney
approved these changes
May 2, 2026
|
Contributor
There was a problem hiding this comment.
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()andsed_escape_replacement()helpers and used them when generating version-migrationsedcommands. - Fixed the
sed -einvocation used for updatingsites-availableconfigs and added a guard to create/etc/enginescript/install-state.confbefore 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")" |
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
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.



returninsed_escape_ere()returninsed_escape_replacement()Original prompt