From 5ebee8ef2a54b8578daf47b17fa82fc85b2cd1cd Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Wed, 17 Jun 2026 21:00:43 +0300 Subject: an0 reject scalar SYNC_DESTINATIONS instead of word-splitting it A scalar SYNC_DESTINATIONS containing spaces was word-split by the shared resolve_config_array helper, breaking a single destination into multiple broken arguments passed to rsync. A list of rsync destinations is inherently a list, and array syntax is the only spelling that preserves embedded spaces. resolve_sync_destinations now detects a scalar declaration via declare -p and fails with a clear config_error telling the user to use array syntax. The array path is unchanged, and resolve_config_array's scalar word-splitting is left intact for TAR_OPTS (where turning "-c -v" into separate options is desired). Adds focused tests proving the scalar case errors without invoking rsync and the array case preserves a space-containing destination as one argument. Updates src/shuriken.default.conf and README.md to document the requirement. Co-Authored-By: Claude Opus 4.8 --- src/lib/config.sync.source.sh | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) (limited to 'src/lib') diff --git a/src/lib/config.sync.source.sh b/src/lib/config.sync.source.sh index 3d516dc..f4df169 100644 --- a/src/lib/config.sync.source.sh +++ b/src/lib/config.sync.source.sh @@ -2,13 +2,37 @@ resolve_sync_destinations() { # destinations_ref is a nameref output filled by resolve_config_array. # shellcheck disable=SC2034 local -n destinations_ref="$1"; shift + local sync_decl - # Parse SYNC_DESTINATIONS (array or scalar) via the shared config-array - # helper. Unlike TAR_OPTS there is no default: an unset or empty value - # simply yields an empty destinations array for callers to validate. - # The helper returns non-zero when SYNC_DESTINATIONS was never declared; - # we ignore that (|| true) since the already-empty array is exactly the - # desired result, and the bare non-zero return would otherwise trip set -e. + # A list of rsync destinations is inherently a list, and the only + # unambiguous spelling is a Bash array: SYNC_DESTINATIONS=( '/path/with + # spaces/' ) preserves embedded spaces, whereas a scalar string would be + # word-split (e.g. "/path/with spaces/" -> two broken destinations passed + # to rsync). So we reject scalar declarations outright instead of silently + # word-splitting them. (TAR_OPTS deliberately keeps scalar word-splitting: + # turning "-c -v" into separate options is the desired behaviour there.) + # + # declare -p fails only when SYNC_DESTINATIONS was never set; that is a + # legitimate "unset" case handled by the || true below, so we skip the + # array-vs-scalar check entirely when the variable does not exist. + if sync_decl=$(declare -p SYNC_DESTINATIONS 2>/dev/null); then + case "$sync_decl" in + declare\ -a*) ;; + *) + config_error \ + 'SYNC_DESTINATIONS must be an array, e.g. SYNC_DESTINATIONS=( '\''dest1'\'' '\''dest2'\'' )' + return 1 + ;; + esac + fi + + # Parse SYNC_DESTINATIONS (always an array at this point) via the shared + # config-array helper. Unlike TAR_OPTS there is no default: an unset or + # empty value simply yields an empty destinations array for callers to + # validate. The helper returns non-zero when SYNC_DESTINATIONS was never + # declared; we ignore that (|| true) since the already-empty array is + # exactly the desired result, and the bare non-zero return would otherwise + # trip set -e. resolve_config_array SYNC_DESTINATIONS destinations_ref || true } -- cgit v1.2.3