Skip to content

hack: fix unquoted variables in hack scripts to prevent word-splitting #9018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ankitsinghsisodya wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −6
Open

hack: fix unquoted variables in hack scripts to prevent word-splitting #9018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfe1342
hack: fix unquoted variables in hack scripts to prevent word-splitting
Ankitsinghsisodya Apr 14, 2026
be0d604
hack: use mapfile array for safe multi-file cp in release.sh
Ankitsinghsisodya Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hack/generate-yamls.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ readonly YAML_LIST_FILE=${2:?"Second argument must be the output file"}
if [[ -z "${YAML_OUTPUT_DIR:-}" ]]; then
readonly YAML_OUTPUT_DIR="$(mktemp -d)"
fi
rm -fr ${YAML_OUTPUT_DIR}/*.yaml
rm -fr "${YAML_OUTPUT_DIR}"/*.yaml

# Generated Knative component YAML files
readonly EVENTING_CORE_YAML=${YAML_OUTPUT_DIR}/"eventing-core.yaml"
Expand Down
9 changes: 5 additions & 4 deletions hack/release.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,13 @@ function build_release() {
# branch since the detail of building may change over time.
local YAML_LIST="$(mktemp)"
export TAG
$(dirname $0)/generate-yamls.sh "${REPO_ROOT_DIR}" "${YAML_LIST}"
ARTIFACTS_TO_PUBLISH=$(cat "${YAML_LIST}" | tr '\n' ' ')
"$(dirname "$0")/generate-yamls.sh" "${REPO_ROOT_DIR}" "${YAML_LIST}"
mapfile -t artifacts < "${YAML_LIST}"
ARTIFACTS_TO_PUBLISH="${artifacts[*]}"
if (( ! PUBLISH_RELEASE )); then
# Copy the generated YAML files to the repo root dir if not publishing.
cp ${ARTIFACTS_TO_PUBLISH} ${REPO_ROOT_DIR}
cp "${artifacts[@]}" "${REPO_ROOT_DIR}"
Comment on lines +68 to +73
Copy link
Copy Markdown
Member

@creydr creydr Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Copy Markdown
Contributor Author

@Ankitsinghsisodya Ankitsinghsisodya Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@creydr The original cp ${ARTIFACTS_TO_PUBLISH} ${REPO_ROOT_DIR} had two issues:

  1. Word splitting on $ARTIFACTS_TO_PUBLISH — that variable holds space-separated filenames. If any filename contains spaces or glob characters, the shell splits it incorrectly before passing it to cp. Using "${artifacts[@]}" (a bash array) passes each element as a distinct argument, safely.

  2. Redundancy — ARTIFACTS_TO_PUBLISH="${artifacts[*]}" (line 70) collapses the array back into a single space-joined string. The new code skips that intermediary and goes straight from the array to cp, which is both simpler and correct.

Copy link
Copy Markdown
Member

@creydr creydr Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ARTIFACTS_TO_PUBLISH needed then? Or you rename artifacts to ARTIFACTS_TO_PUBLISH to make it a bit clearer

fi
}

main $@
main "$@"
2 changes: 1 addition & 1 deletion hack/verify-codegen.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export GO111MODULE=on

source $(dirname $0)/../vendor/knative.dev/hack/library.sh

readonly TMP_DIFFROOT="$(mktemp -d ${REPO_ROOT_DIR}/tmpdiffroot.XXXXXX)"
readonly TMP_DIFFROOT="$(mktemp -d "${REPO_ROOT_DIR}/tmpdiffroot.XXXXXX")"

cleanup() {
rm -rf "${TMP_DIFFROOT}"
Expand Down
Loading