feat: allow multiple hotfixes and add 10.0.2.2 #69
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the hotfix system to support multiple hotfixes per version by changing the hotfixes.json structure from objects to arrays, and adds a second hotfix (10-0-2-2) for version 10.0.2. Additionally, it improves the Docker cleanup process in the build pipeline.
Changes:
- Modified
hotfixes.jsonstructure to use arrays instead of objects, enabling multiple hotfixes per version - Updated
download-release.shto iterate over multiple hotfixes for each version - Enhanced
update-multiplatform-images.shwith bootstrap command and improved cleanup logic
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| hotfixes.json | Restructured from object-per-version to array-per-version format; added second hotfix (10-0-2-2) for version 10.0.2 |
| download-release.sh | Added loop logic to process multiple hotfixes per version; changed from single hotfix to array iteration |
| update-multiplatform-images.sh | Added buildx bootstrap command; improved Docker cleanup with more targeted container/image removal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| docker ps -a | awk '{ print $1,$2 }' | grep "${IMAGE_BASE}" | awk '{print $1 }' | xargs -I {} docker rm {} | ||
| docker images | awk '{ print $1,$3 }' | grep "${IMAGE_BASE}" | awk '{print $2 }' | xargs -I {} docker rmi {} --force | ||
| docker buildx stop && docker buildx rm | ||
| docker rm $(docker ps --filter status=exited -q) || true |
There was a problem hiding this comment.
The command substitution $(docker ps --filter status=exited -q) could fail if there are no exited containers, causing docker rm to receive an empty argument. While the || true catches the error, it would be cleaner to use xargs with the -r flag like the previous line, or check if the result is non-empty before calling docker rm: docker ps --filter status=exited -q | xargs -r docker rm || true
| docker rm $(docker ps --filter status=exited -q) || true | |
| docker ps --filter status=exited -q | xargs -r docker rm || true |
| { | ||
| "original_files": [ | ||
| "idsvr/lib/identityserver.core-10.0.2-1ec50d3ecd.jar", | ||
| "idsvr/lib/identityserver.usermanagement-10.0.2-1ec50d3ecd.jar" | ||
| ], | ||
| "hotfix_path": "10-0-2-2" |
There was a problem hiding this comment.
The PR title mentions "add 10.0.2.2" but the hotfix_path is "10-0-2-2" (with dashes instead of dots). While this naming convention is consistent with other hotfix paths in the file, there's a discrepancy between the PR title and the actual implementation. The version key is "10.0.2" and the second hotfix for that version has the path "10-0-2-2", which suggests this is hotfix #2 for version 10.0.2, not a new version 10.0.2.2.
| HOTFIX_PATH=$(jq -e -r '."'$VERSION'".hotfix_path' hotfixes.json) | ||
| # Iterate over all hotfixes for $VERSION (array) | ||
| HOTFIX_COUNT=$(jq -r '."'$VERSION'" | length' hotfixes.json) | ||
| for ((i=0; i<$HOTFIX_COUNT; i++)); do |
There was a problem hiding this comment.
The bash arithmetic expansion syntax is not POSIX-compliant. While this works in bash, the loop variable initialization should use proper quoting. Consider using for ((i=0; i<HOTFIX_COUNT; i++)) instead to ensure the variable HOTFIX_COUNT is properly evaluated even if it contains whitespace or special characters. Additionally, there's a potential issue if HOTFIX_COUNT is not a valid integer (e.g., if jq returns an error or unexpected value), which would cause the loop to fail. Consider adding validation: HOTFIX_COUNT=$(jq -r '."'$VERSION'" | length' hotfixes.json) && [[ "$HOTFIX_COUNT" =~ ^[0-9]+$ ]] || HOTFIX_COUNT=0
| for ((i=0; i<$HOTFIX_COUNT; i++)); do | |
| if ! [[ "$HOTFIX_COUNT" =~ ^[0-9]+$ ]]; then | |
| HOTFIX_COUNT=0 | |
| fi | |
| for ((i=0; i<HOTFIX_COUNT; i++)); do |
| # Iterate over all hotfixes for $VERSION (array) | ||
| HOTFIX_COUNT=$(jq -r '."'$VERSION'" | length' hotfixes.json) | ||
| for ((i=0; i<$HOTFIX_COUNT; i++)); do | ||
| HOTFIX_PATH=$(jq -r '."'$VERSION'"['$i'].hotfix_path' hotfixes.json) |
There was a problem hiding this comment.
The variable $i is not quoted in the jq array index access. While this typically works in bash, it's a best practice to quote variables to prevent issues with whitespace or special characters. Change to: HOTFIX_PATH=$(jq -r '."'$VERSION'"['"$i"'].hotfix_path' hotfixes.json)
| for original_file in $(jq -e -r '."'$VERSION'".original_files[]' hotfixes.json); do | ||
| rm "${UNPACK_DIR}/${original_file}" | ||
| done | ||
| for original_file in $(jq -r '."'$VERSION'"['$i'].original_files[]' hotfixes.json); do |
There was a problem hiding this comment.
The variable $i is not quoted in the jq array index access. While this typically works in bash, it's a best practice to quote variables to prevent issues with whitespace or special characters. Change to: for original_file in $(jq -r '."'$VERSION'"['"$i"'].original_files[]' hotfixes.json); do
| done | ||
|
|
||
| tar -xzf "${HOTFIX_PATH}-${VERSION}.tgz" --exclude='*.md' -C "$UNPACK_DIR" | ||
| tar -xzf "${HOTFIX_PATH}-${VERSION}.tgz" --exclude='*.md' -C "$UNPACK_DIR" |
There was a problem hiding this comment.
The tar -xzf "${HOTFIX_PATH}-${VERSION}.tgz" --exclude='*.md' -C "$UNPACK_DIR" extraction runs on a remotely downloaded hotfix archive without validating or normalizing entry paths, which allows a malicious tar (e.g., with ../ segments) to write files outside of $UNPACK_DIR (path traversal). An attacker who can influence the hotfix tarball or the release API response could overwrite arbitrary files on the build host with the privileges of the script, potentially leading to code execution via modified build artifacts or other system files. To mitigate this, ensure that all paths from the archive are sanitized/rejected if they are absolute or contain .. segments, or use a safer extraction mechanism that enforces confinement to $UNPACK_DIR.
No description provided.