(#9400 P1b) Replace unsafe eval with declare -g and nameref

[iav]

Summary

Replaces 5 eval usages that can be trivially eliminated — part of the bash safety plan in #9400 (P1b).

File	Old	New
lib/functions/cli/utils-cli.sh	eval "declare -g $name=\"$value\""	declare -g "${name}=${value}"
lib/functions/configuration/interactive.sh	eval "$1"='$2'	declare -g "${1}=${2}"
lib/functions/configuration/interactive.sh	eval "ARMBIAN_INTERACTIVE_CONFIGS[${1}]"='$2'	ARMBIAN_INTERACTIVE_CONFIGS["${1}"]="${2}"
lib/functions/configuration/change-tracking.sh	eval "var_value=\"\${${var_name}[@]}\""	local -n nameref
lib/functions/artifacts/artifacts-obtain.sh	eval "declare ${var}_ARRAY=\"\${${var}[*]}\""	local -n nameref

Why eval is risky here

eval executes its argument as shell code. If a variable name or value were ever a crafted string (e.g. from user input or a config file), it could inject arbitrary commands. The bash builtins used as replacements do not have this property.

Note: parse_cmdline_params already validates parameter names against ^[a-zA-Z_][a-zA-Z0-9_]*$ before inserting into ARMBIAN_PARSED_CMDLINE_PARAMS, so names reaching apply_cmdline_params_to_env are guaranteed to be valid identifiers — closing the array-index injection vector at the source.

The nameref case explained

change-tracking.sh and artifacts-obtain.sh both needed to read an array variable whose name is stored in a scalar at runtime. The original comment in change-tracking said # sorry. The fix uses bash 4.3+ nameref (local -n), which creates an alias to the variable without executing any code:

# Before — eval reads array by dynamic name, code-injection risk:
eval "var_value=\"\${${var_name}[@]}\"" # sorry

# After — nameref creates a safe alias, no code executed:
local -n _ct_arr_ref="${var_name}"  # alias for the array named in $var_name
var_value="${_ct_arr_ref[*]}"       # read through the alias
unset -n _ct_arr_ref                # remove alias only (not the array!) to avoid
                                    # "already a nameref" warnings next iteration

Out of scope

Other eval usages in lib/ intentionally left alone:

• cli-configdump.sh:61 — deserializer for @Q-quoted serialization (safe by construction, cannot be trivially replaced without reworking serialization).
• Code/hook materialization (extensions.sh, armbian-bsp-desktop-deb.sh, traps.sh, heredocs in utils-cli.sh) — legitimate dynamic code generation.
• eval env "${COMPILER}gcc" in compilation paths — redundant eval but no injection risk; not in P1b scope.

Test plan

• shellcheck passes with no new warnings
• compile.sh runs normally (cmdline param application, interactive config, change tracking, artifact config dump all exercised in normal builds)

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

These changes replace unsafe eval-based dynamic variable assignments with safer bash mechanisms across three shell script utility files, maintaining functional behavior while improving security and code safety.

Changes

Cohort / File(s)	Summary
Shell Script Security Hardening lib/functions/cli/utils-cli.sh	Replaces eval with direct declare -g syntax for global variable assignment from cmdline parameters.
Array Variable Extraction lib/functions/configuration/change-tracking.sh	Replaces eval-based array value extraction with bash nameref approach for safer variable aliasing.
Interactive Configuration lib/functions/configuration/interactive.sh	Replaces eval-based assignment with declare -g combined with direct associative array updates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hop through bash, away from eval's peril,
Where nameref and declare now bravely stir!
No more dancing quotes in dangerous ways,
Our scripts are safer through these careful days! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main changes: replacing unsafe eval calls with safer alternatives (declare -g and nameref) across multiple shell script files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/p1b-eval-to-declare

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[iav]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[EvilOlaf]

The comment tells me that Ricardo put some thought into this before leaving as it is. So I'd like to hear his opinion on the change.

[igorpecovnik]

@rpardini

[iav]

@rpardini ?