(#9400 P3b,P3c) read -r and safe array assignment#9406
Open
iav wants to merge 2 commits intoarmbian:mainfrom
Open
(#9400 P3b,P3c) read -r and safe array assignment#9406iav wants to merge 2 commits intoarmbian:mainfrom
iav wants to merge 2 commits intoarmbian:mainfrom
Conversation
Other read calls in the same file already use -r. Without -r, backslashes in user input are interpreted as escape characters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rd splitting
Replace unquoted array assignment `arr=(${var})` with
`IFS=' ' read -ra arr <<< "${var}"` to prevent glob expansion.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
github-actions
bot
added
size/small
Contributor
📝 WalkthroughWalkthroughTwo shell script files receive refinements: bootscript template rendering and kernel pin extraction parsing improvements in the BSP CLI module, plus a read flag adjustment in the patch naming flow to prevent backslash escape interpretation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
Needs review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (1)
235-236: Good fix for glob-safe array assignment.Line 236 correctly prevents glob expansion for values containing
*or?.Note: Line 235 still uses unquoted
$KERNEL_UPGRADE_FREEZEin the$(echo $KERNEL_UPGRADE_FREEZE | sed ...)command substitution, which has the same glob expansion risk this PR aims to fix. Consider quoting it as"$KERNEL_UPGRADE_FREEZE"for consistency.Proposed fix
- for pin_variants in $(echo $KERNEL_UPGRADE_FREEZE | sed "s/,/ /g"); do + for pin_variants in $(echo "$KERNEL_UPGRADE_FREEZE" | sed "s/,/ /g"); do
iav
commented
Feb 16, 2026
| @@ -233,7 +233,7 @@ function compile_armbian-bsp-cli() { | |||
| # if freeze variable is removed, upgrade becomes possible again | |||
| if [[ "${BETA}" != "yes" ]]; then | |||
Contributor
Author
There was a problem hiding this comment.
@igorpecovnik @rpardini Question about the existing logic around this code:
if [[ "${BETA}" != "yes" ]]; then
for pin_variants in ...; do
...
done
else
touch "${destination}"/etc/apt/preferences.d/frozen-armbian
fiIs this BETA check intentional here? The current behavior:
BETA=yes(nightly) →touchcreates an emptyfrozen-armbian(no actual pinning)BETA=no(stable) → freeze logic runs, pins are writtenBETAunset (user-built) → freeze logic runs, butKERNEL_UPGRADE_FREEZEis typically empty so loop does nothing
The empty touch in the else branch for nightly builds seems like a no-op — dpkg-deb includes the 0-byte file, but it has no effect on apt.
Was this designed this way, or should the condition be something like [[ -n "${KERNEL_UPGRADE_FREEZE}" ]] instead of checking BETA?
Contributor
Author
|
Re: @coderabbitai nitpick about unquoted |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
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
1 participant
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.
Summary
patching.sh: add missing-rflag toread -e -p— without it, backslashes in user input are interpreted as escape characters. Otherreadcalls in the same file already use-r.armbian-bsp-cli-deb.sh: replace unquotedarr=(${var})withIFS=' ' read -ra arr <<< "${var}"— prevents glob expansion on values containing*or?.Test plan
userpatch_createinteractive prompt still works correctly with backslash-containing inputKERNEL_UPGRADE_FREEZEvalues🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor