Proposal: Improve Bash syntax safety across the codebase

[iav]

Proposal: Improve Bash syntax safety across the codebase

Question for maintainers

The Armbian build framework requires Bash 5.x and already follows many modern Bash best
practices ([[ ]], mapfile, read -r, associative arrays). However, a codebase audit
has revealed ~100 places where legacy or unsafe syntax patterns remain. These patterns
carry risks ranging from word-splitting bugs to potential code injection via eval.

Should the project invest in a systematic cleanup of these patterns?

The changes are mechanical, low-risk, and can be done incrementally per group.

---

Proposed change groups (by priority)

Check the groups you approve for implementation:

[…] P0 — Unquoted variables in destructive commands (~15 places) #9401

Variables without double quotes passed to rm, mv, cp, mount, umount, losetup,
sed -i. Word splitting on any of these can cause data loss or corruption.

File	Line	Code
extensions/image-output-abl.sh	30	mount ${ROOTFS_IMAGE_FILE} ${new_rootfs_image_mount_dir}
extensions/image-output-abl.sh	32	umount ${old_rootfs_image_mount_dir}
extensions/image-output-abl.sh	33	losetup -d ${old_image_loop_device}
extensions/image-output-abl.sh	34	rm ${DESTIMG}/${version}.img
extensions/image-output-abl.sh	36	sed -i ... ${new_rootfs_image_mount_dir}/etc/fstab
extensions/uwe5622-allwinner.sh	15	chroot $SDCARD /bin/bash -c ...
extensions/uwe5622-allwinner.sh	19-22	mkdir -p $destination/..., cp $SRC/... $destination/...
extensions/cloud-init/cloud-init.sh	47,53	cp ${config_src}/* $config_dst
extensions/allwinner-kernel-bump.sh	80,84,85	cp ${dir}/* ${dir}, mv ${dir}/... ${dir}/...
extensions/lvm.sh	72-73	e2label /dev/mapper/${LVM_VG_NAME}-root, grep ${LVM_VG_NAME}
lib/functions/image/partitioning.sh	287	rm -f $SDCARD/etc/fstab
lib/functions/image/partitioning.sh	331	mv ${cryptroot_autounlock_key_file:?} ${SDCARD}${luks_key_file}
lib/functions/image/partitioning.sh	470	rm $SDCARD/boot/armbianEnv.txt
lib/functions/image/partitioning.sh	504,506	echo ... >> $SDCARD/boot/extlinux/...

Fix: wrap every variable in "${VAR}". For glob expansion keep * outside quotes: "${dir}"/*.

---

[ ] P1a — Single brackets [ ] to [[ ]] (~32 places)

[ ] is POSIX test — vulnerable to word splitting, glob expansion, and does not support
&&, ||, =~ or pattern matching. The project already requires Bash 5.x, so [[ ]]
should be used everywhere.

Highest-risk instances (unquoted variables inside [ ]):

File	Line	Code
lib/functions/image/partitioning.sh	485	if [ -z ${BOOTSCRIPT_OUTPUT} ];
lib/functions/bsp/armbian-bsp-cli-deb.sh	350	if [ -f /boot/${BOOTSCRIPT_DST} ];
config/sources/families/rockchip.conf	229	if [ -f $1/u-boot-rockchip.bin ];
config/sources/families/spacemit.conf	64	if [ -b ${2}boot0 ];
extensions/image-output-abl.sh	8,13	if [ ! -z "$VAR" ]; (anti-pattern)
extensions/image-output-abl.sh	40	if [ ${#ABL_DTB_LIST[@]} -ne 0 ];

Medium-risk (quoted variables, but still [ ]):

File	Line	Code
lib/functions/compilation/uboot.sh	562	if [ "${FORCE_UBOOT_UPDATE:-no}" == "yes" ];
lib/functions/host/prepare-host.sh	44	if [ "$OFFLINE_WORK" == "yes" ];
lib/functions/main/config-prepare.sh	170	if [ "${KERNEL_DRIVERS_SKIP[*]}" == "" ];
lib/functions/configuration/main-config.sh	26,601, 605,620, 641	various [ -f ... ], [ -n ... ]
lib/functions/rootfs/distro-specific.sh	82,114, 141,206	various [ -f ... ], [ -e ... ]
lib/functions/rootfs/distro-agnostic.sh	568	if [ -d ... ] && [ -n ... ];
lib/functions/image/partitioning.sh	251,270	if [ "$num" -ge "24100" ];
lib/functions/image/initrd.sh	35	if [ "$target_dir" != "" ];
lib/functions/image/fingerprint.sh	31	if [ -n "$2" ];
lib/functions/bsp/armbian-bsp-cli-deb.sh	325,361, 394,451, 456,467, 470,473, 476,480	various
lib/functions/bsp/utils-bsp.sh	27	if [ -d ... ];
lib/functions/compilation/utils-compilation.sh	28	if [ -f localversion-next ];
extensions/lowmem.sh	16,38	if [ ! -f ... ];
extensions/gxlimg.sh	63,104	if [ -e ... ], if [ ! -s ... ]

Fix: [ x ] → [[ x ]]; [ ! -z "$x" ] → [[ -n "${x}" ]]; [ "$x" == "y" ] → [[ "${x}" == "y" ]].

Also note: == inside [ ] is a bashism not supported by POSIX test — another reason to use [[ ]].

---

[ ] P1b — Unsafe eval replaceable with Bash 5.x builtins (~2-3 places)

Most eval usages in the codebase are part of the extension/artifact code-generation
machinery and are hard to replace. However, a few can be trivially eliminated:

File	Line	Current	Replacement
lib/functions/configuration/interactive.sh	33-34	eval "$1"='$2'	declare -g "$1"="$2" or printf -v "$1" '%s' "$2"
lib/functions/compilation/uboot.sh	339,357	eval "${_old_nullglob}"	Store via shopt -p nullglob, restore with direct shopt call

Remaining eval usages (not proposed for change — would require architectural rework):

File	Line	Purpose
lib/functions/artifacts/artifacts-obtain.sh	32,49	Heredoc-based function generation
lib/functions/general/extensions.sh	302	Hook context variable injection
lib/functions/logging/traps.sh	161	Cleanup handler dispatch
lib/functions/host/mountpoints.sh	70,93	Docker mountpoint dict expansion
lib/functions/compilation/packages/armbian-desktop-deb.sh	57	Aggregated package code
lib/functions/bsp/armbian-bsp-desktop-deb.sh	59	Aggregated BSP code
lib/functions/cli/utils-cli.sh	107,121	Dynamic CLI dispatch

---

[ ] P2a — for var in $(echo ... | tr/sed) to parameter expansion or mapfile (~9 places)

Spawning echo | tr subprocesses just to split a comma-separated string. Bash can do this
natively.

File	Line	Current	Replacement
lib/functions/general/extensions.sh	144	for x in $(echo "${VAR}" | tr "," " ")	IFS=',' read -ra arr <<< "${VAR}"
lib/functions/configuration/interactive.sh	221	for x in $(echo "${KERNEL_TARGET}" | tr "," "\n")	same
lib/functions/bsp/armbian-bsp-cli-deb.sh	235	for x in $(echo $VAR | sed "s/,/ /g")	for x in ${VAR//,/ }
lib/functions/rootfs/distro-agnostic.sh	517	for i in $(echo "${SERIALCON}" | sed "s/,/ /g")	same
lib/functions/main/build-packages.sh	73	for x in $(tr ',' ' ' <<< "${CLEAN_LEVEL}")	for x in ${CLEAN_LEVEL//,/ }
lib/functions/compilation/utils-compilation.sh	67,72	for dir in $(< /tmp/file)	mapfile -t dirs < /tmp/file
lib/functions/general/countdown.sh	27,57	for i in $(seq 1 "${loops}")	for ((i=1; i<=loops; i++))

---

[ ] P2b — echo "$var" | cmd to here-strings or parameter expansion (~27 places)

Unnecessary subshell from echo piped into grep/sed/cut/awk. Replace with
<<< here-strings or native ${var//pattern/replacement}.

Largest cluster (7 identical patterns in one file):

File	Lines	Current	Replacement
lib/functions/host/docker.sh	165,168,171,174,177,184,195	echo "${DOCKER_INFO}" | grep -i "Key:" | cut -d: -f2 | xargs echo -n	grep -i "Key:" <<< "${DOCKER_INFO}" | cut -d: -f2 | xargs echo -n

Other places:

File	Line	Current	Replacement
lib/functions/general/git-ref2info.sh	135,143,151	echo "${git_source}" | cut -d/ -f4-5	"${git_source}" parameter expansion
lib/functions/general/git.sh	89	echo "$url" | sed "s|...|...|"	"${url/https:\/\/github.com\//${GITHUB_SOURCE}/}"
lib/functions/compilation/uboot.sh	566	echo $root_partition | sed 's/\/dev\///g'	"${root_partition#/dev/}"
lib/functions/general/hash-files.sh	75,98,116	echo "${hash}" | sha256sum	sha256sum <<< "${hash}"
lib/functions/general/python-tools.sh	45	echo "${ver}" | awk | cut	parameter expansion
lib/functions/image/partitioning.sh	250	echo "$ver" | awk -F.	parameter expansion

---

[🏗️] P3a — Useless cat (~8 places) #9404

cat file | cmd replaced with direct file arguments or < file (one fewer process).

File	Line	Current	Applied fix
lib/functions/logging/trap-logging.sh	62	cat file | gzip > out	gzip -c file > out
lib/functions/logging/trap-logging.sh	101	cat file | ansi2txt >> out	ansi2txt < file >> out
lib/functions/logging/logging.sh	99	cat file | grep | sed	grep file | sed
lib/functions/image/initrd.sh	73	cat file | md5sum	md5sum file
lib/functions/image/write-device.sh	21	cat file | awk	awk file
lib/functions/logging/export-logs.sh	161	$(cat file | sed)	$(sed file)
lib/functions/general/python-tools.sh	65	"$(cat file)"	"$(< file)"
lib/functions/artifacts/artifact-armbian-base-files.sh	117	cat file | grep -vP	grep -vP file

---

[ ] P3b — Minor: read -e without -r (1 place)

File	Line	Current	Replacement
lib/functions/compilation/patch/patching.sh	217	read -e -p "Patch Subject: "	read -r -e -p "Patch Subject: "

Other read calls in the same file (lines 159, 172, 191) already use -r.

---

[ ] P3c — Unquoted word-splitting array assignment (1 place)

File	Line	Current	Replacement
lib/functions/bsp/armbian-bsp-cli-deb.sh	236	extracted_pins=(${pin_variants//@/ })	IFS=' ' read -ra extracted_pins <<< "${pin_variants//@/ }"

---

Scope explicitly excluded

The following patterns were reviewed and intentionally not proposed for change:

• [[ -z $var ]] without quotes inside [[ ]] (~21 places) — [[ ]] prevents word
  splitting by design; quoting is a style preference, not a safety issue.
• eval in extension/artifact machinery (~10 places) — these are core architectural
  patterns for dynamic code generation; replacing them would require significant rework
  with no clear safety benefit since inputs are framework-controlled.
• for item in $SPACE_SEPARATED_LIST where the variable is intentionally space-delimited
  (e.g., $SRC_CMDLINE, $UBOOT_TARGET_MAP) — word splitting is the desired behavior.
• echo ... | sha256sum — the <<< here-string alternative appends a trailing newline
  which changes the hash; keeping echo is correct here.

---

Implementation plan

Each approved group would be a separate commit (or PR if preferred), making review and
bisection straightforward. All changes are mechanical and testable with shellcheck.

[github-actions]

Jira ticket: AR-2818

[rpardini]

Those all sound good -- ofc the devil is in the details.... but really seems fine