fix(tools): guard indirect variable expansion against empty env_var

[wuhuizuo]

Problem

tools/validate-repair-zot-image.sh crashes with:

./validate-repair-zot-image.sh: line 173: : invalid variable name

Root cause

In prompt_required(), the line val="${!env_var:-}" performs indirect variable expansion. When callers pass an empty string as env_var (e.g. prompt_required "some prompt" ""), this expands to ${!:-} — trying to dereference a variable with an empty name, which is invalid in bash.

Fix

Guard the indirect expansion with a check for non-empty env_var before attempting expansion.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign wuhuizuo for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary:
This PR fixes a bug in tools/validate-repair-zot-image.sh where indirect variable expansion fails if the environment variable name is empty, causing a script crash. The fix adds a guard to check that the variable name is non-empty before expanding it. The approach is straightforward and addresses the root cause precisely. The patch is clean and focused, improving robustness with minimal code changes.

---

Code Improvements

• Guarding indirect expansion with explicit check:

  • File: tools/validate-repair-zot-image.sh
  • Line: 170-180 (around the prompt_required() function)
  • Issue: Previously, val="${!env_var:-}" was executed without confirming env_var was non-empty, resulting in invalid bash syntax.
  • Suggestion: The added check if [[ -n "${env_var:-}" ]]; then is correct and prevents the crash.
  • Further improvement: Consider enhancing readability by explicitly handling the empty env_var case with a clear error message or a fallback to avoid silent failures or unexpected behavior when env_var is empty.

  Example:

  if [[ -z "$env_var" ]]; then
      error "Environment variable name is empty in prompt_required"
      return 1
  fi
  val="${!env_var:-}"
  if [[ -n "$val" ]]; then
      info "Using $env_var from environment."
      echo "$val"
      return
  fi

Best Practices

• Error handling for empty env_var:

  • File: tools/validate-repair-zot-image.sh
  • Line: 170-180
  • Why: Currently, if env_var is empty, the function simply skips the indirect expansion and continues prompting. This may hide the fact that the caller passed an invalid parameter.
  • Suggestion: Add a warning or error log to surface this potential misuse, helping future debugging and enforcing correct usage.

• Documentation:

  • File: tools/validate-repair-zot-image.sh
  • Line: function prompt_required()
  • Why: The function's parameters and behavior (especially about env_var) are not documented.
  • Suggestion: Add comments describing the function, its parameters, and expected behavior, including the handling of empty env_var.

• Testing coverage:

  • Why: The PR fixes a crash triggered by calling prompt_required with an empty environment variable name.
  • Suggestion: Add a test case for this edge case to ensure the function behaves gracefully when env_var is empty, preventing regression.

---

Overall, the PR correctly fixes the immediate crash issue with a minimal and clear change. Adding explicit error handling and documentation for the edge case of empty env_var would further improve code robustness and maintainability.