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 <noreply@anthropic.com>

5 files changed, 152 insertions, 12 deletions

diff --git a/README.md b/README.md
index 9e9dc81..b853260 100644
--- a/README.md
+++ b/README.md
@@ -204,6 +204,11 @@ SYNC_DESTINATIONS=(
 )
 ```
 
+`SYNC_DESTINATIONS` must be a Bash array, even for a single destination (for
+example `SYNC_DESTINATIONS=( '/path/with spaces/' )`). A scalar string is
+rejected with an error, since word-splitting would break destinations that
+contain spaces.
+
 `--sync` runs `rsync -av --delete "$DIST_DIR/" "$destination"` for each
 destination by default. The trailing slash on `DIST_DIR/` means the generated
 contents are copied into the target directory. Set `SYNC_DELETE=no` or pass
diff --git a/bin/shuriken b/bin/shuriken
index 216460c..fd922e9 100755
--- a/bin/shuriken
+++ b/bin/shuriken
@@ -4567,13 +4567,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
+
+    # 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 (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.
+    # 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
 }
 
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
 }
 
diff --git a/src/shuriken.default.conf b/src/shuriken.default.conf
index 9d0b1a8..d71ef8e 100644
--- a/src/shuriken.default.conf
+++ b/src/shuriken.default.conf
@@ -42,6 +42,9 @@ TAR_OPTS=(-c)
 # Destinations for `shuriken --sync`. Each destination is passed to rsync
 # with DIST_DIR/ as the source, so the generated contents are copied into the
 # target directory. Sync deletes removed files from destinations by default.
+# SYNC_DESTINATIONS must be a Bash array, even for a single destination, e.g.
+# SYNC_DESTINATIONS=( '/path/with spaces/' ). A scalar string is rejected with
+# an error because word-splitting would break destinations containing spaces.
 SYNC_DELETE=yes
 SYNC_DESTINATIONS=(
 #    admin@fishfinger.buetow.org:/var/www/htdocs/example.org/
diff --git a/tests/cli.sh b/tests/cli.sh
index a16cbea..a28c1f1 100755
--- a/tests/cli.sh
+++ b/tests/cli.sh
@@ -5494,6 +5494,84 @@ test_sync_rejects_missing_dist() {
     test::teardown
 }
 
+test_sync_rejects_scalar_destinations() {
+    # Regression for an0: a scalar SYNC_DESTINATIONS with spaces used to be
+    # word-split into multiple broken rsync destinations. We now reject scalar
+    # declarations outright so the only spelling is an array, where embedded
+    # spaces are preserved naturally.
+    local config_file
+    local dist_dir
+    local fake_bin
+    local output
+    local rsync_log
+
+    test::setup
+    fake_bin="$TEST_TMPDIR/bin"
+    config_file="$TEST_TMPDIR/shuriken.conf"
+    dist_dir="$TEST_TMPDIR/dist"
+    rsync_log="$TEST_TMPDIR/rsync.log"
+
+    test::install_rsync_spy "$fake_bin"
+    mkdir -p "$dist_dir"
+    printf 'generated\n' > "$dist_dir/index.html"
+    {
+        printf 'DIST_DIR=%q\n' "$dist_dir"
+        printf 'SYNC_DESTINATIONS=%q\n' '/path/with spaces/'
+    } > "$config_file"
+
+    output=$(
+        cd "$TEST_TMPDIR"
+        PATH="$fake_bin:$PATH" TEST_RSYNC_LOG="$rsync_log" \
+            test::capture_failure_output "$TEST_SHURIKEN" --sync
+    )
+
+    test::assert_contains \
+        'ERROR: SYNC_DESTINATIONS must be an array' \
+        "$output"
+    # rsync must never run: the scalar was rejected, not word-split.
+    test::assert_path_absent "$rsync_log"
+    test::teardown
+}
+
+test_sync_array_destination_preserves_spaces() {
+    # A single array destination containing spaces must reach rsync intact as
+    # one argument, proving the array path is unaffected by the an0 fix.
+    local config_file
+    local dist_dir
+    local fake_bin
+    local rsync_log
+    local rsync_output
+
+    test::setup
+    fake_bin="$TEST_TMPDIR/bin"
+    config_file="$TEST_TMPDIR/shuriken.conf"
+    dist_dir="$TEST_TMPDIR/dist"
+    rsync_log="$TEST_TMPDIR/rsync.log"
+
+    test::install_rsync_spy "$fake_bin"
+    mkdir -p "$dist_dir"
+    printf 'generated\n' > "$dist_dir/index.html"
+    {
+        printf 'DIST_DIR=%q\n' "$dist_dir"
+        printf 'SYNC_DESTINATIONS=(%q)\n' '/path/with spaces/'
+    } > "$config_file"
+
+    (
+        cd "$TEST_TMPDIR"
+        PATH="$fake_bin:$PATH" TEST_RSYNC_LOG="$rsync_log" \
+            "$TEST_SHURIKEN" --sync
+    )
+
+    rsync_output=$(<"$rsync_log")
+    # argc=4 (-av, --delete, source, single destination): the destination is
+    # NOT split into two arguments despite the embedded space.
+    # The rsync spy logs each argument with %q quoting, so a single argument
+    # with a space appears as one escaped arg3 (not two split args).
+    test::assert_contains 'argc=4' "$rsync_output"
+    test::assert_contains 'arg3=/path/with\ spaces/' "$rsync_output"
+    test::teardown
+}
+
 test_positional_commands_fail_without_deprecation() {
     local output
     local old_command
@@ -6125,6 +6203,12 @@ main() {
         '--sync rejects missing dist' \
         test_sync_rejects_missing_dist
     test::run_case \
+        '--sync rejects scalar destinations' \
+        test_sync_rejects_scalar_destinations
+    test::run_case \
+        '--sync array destination preserves spaces' \
+        test_sync_array_destination_preserves_spaces
+    test::run_case \
         'positional commands fail without deprecation output' \
         test_positional_commands_fail_without_deprecation
     test::run_case \