From 9beaf6dfc0ad9b73c5ed1854f18decd222a5e847 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 18:38:35 +0900 Subject: [PATCH 01/12] feat(rolling-update): forward EXTRA_ENV as docker -e flags Lets deploy.env expose tunable flags without editing this script. The immediate trigger is ELASTICKV_RAFT_DISPATCHER_LANES=1, a feature flag added in elastickv PR #577 that routes raft messages through 4 per-peer lanes (heartbeat/replication/snapshot/other) to keep heartbeats from starving under MsgApp/MsgSnap bursts. We want to flip it on without rebuilding or shipping a new image. EXTRA_ENV is a whitespace-separated KEY=VALUE list; each pair becomes a single docker run -e flag. Values may contain characters bash would otherwise interpret, but pairs themselves must not contain whitespace. --- scripts/rolling-update.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 85ccd48cd..8df98b0f7 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -665,12 +665,29 @@ run_container() { ) fi + # Pass through additional container environment variables from EXTRA_ENV. + # Accepts a whitespace-separated list of KEY=VALUE pairs, e.g.: + # EXTRA_ENV="ELASTICKV_RAFT_DISPATCHER_LANES=1 ELASTICKV_PEBBLE_CACHE_MB=512" + # Each pair is forwarded as a single `-e KEY=VALUE` flag so VALUE may contain + # characters that bash would otherwise interpret; pairs themselves must not + # contain whitespace. + local extra_env_flags=() + if [[ -n "${EXTRA_ENV:-}" ]]; then + # shellcheck disable=SC2206 + local -a extra_env_pairs=( ${EXTRA_ENV} ) + local pair + for pair in "${extra_env_pairs[@]}"; do + extra_env_flags+=(-e "$pair") + done + fi + docker run -d \ --name "$CONTAINER_NAME" \ --restart unless-stopped \ --network host \ -v "$DATA_DIR:$DATA_DIR" \ "${s3_creds_volume[@]}" \ + "${extra_env_flags[@]}" \ "$IMAGE" "$SERVER_ENTRYPOINT" \ --address "${NODE_HOST}:${RAFT_PORT}" \ --redisAddress "${NODE_HOST}:${REDIS_PORT}" \ From 58e6508f3b3a6d1f2dd15b6f32963afe94862f5b Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 18:53:50 +0900 Subject: [PATCH 02/12] fix(rolling-update): forward EXTRA_ENV through SSH env dispatch The SSH invocation forwards a fixed list of variables via `env KEY=VAL bash -s` with a QUOTED heredoc, so `EXTRA_ENV` referenced by `run_container` inside the remote block was never populated. Add it to the env pre-command with a default empty value so run_container's existing null-check works when deploy.env doesn't set it. --- scripts/rolling-update.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 8df98b0f7..67fec9664 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -419,6 +419,7 @@ update_one_node() { ALL_NODE_HOSTS_CSV="$all_node_hosts_csv" \ RAFT_TO_REDIS_MAP="$RAFT_TO_REDIS_MAP" \ RAFT_TO_S3_MAP="$RAFT_TO_S3_MAP" \ + EXTRA_ENV="${EXTRA_ENV:-}" \ bash -s <<'REMOTE' set -euo pipefail From 350acdc5ea391d9d662a38885b9d1cdc9422b0de Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 18:54:11 +0900 Subject: [PATCH 03/12] fix(rolling-update): glob-safe EXTRA_ENV split and validate KEY=VALUE Switch from an unquoted array expansion to `read -r -a` when splitting EXTRA_ENV so values containing glob metacharacters (*, ?) are not expanded against the control host's cwd. Also reject entries that do not match KEY=VALUE with a non-empty KEY, since docker treats `-e NAME` (copy from environment) differently from `-e NAME=value` and a malformed pair would silently behave unexpectedly. --- scripts/rolling-update.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 67fec9664..71f8367fe 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -674,10 +674,15 @@ run_container() { # contain whitespace. local extra_env_flags=() if [[ -n "${EXTRA_ENV:-}" ]]; then - # shellcheck disable=SC2206 - local -a extra_env_pairs=( ${EXTRA_ENV} ) + # Split on whitespace without triggering filename globbing. + local -a extra_env_pairs=() + read -r -a extra_env_pairs <<< "${EXTRA_ENV}" local pair for pair in "${extra_env_pairs[@]}"; do + if [[ "$pair" != *=* || "$pair" == =* ]]; then + echo "invalid EXTRA_ENV entry '$pair'; expected KEY=VALUE" >&2 + exit 1 + fi extra_env_flags+=(-e "$pair") done fi From b8a448c719b3397c4854fa38ad62bde14c034b9f Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 18:54:31 +0900 Subject: [PATCH 04/12] docs(rolling-update): document EXTRA_ENV in usage() Add an EXTRA_ENV entry to the Optional environment section of the script's help text, with the expected whitespace-separated KEY=VALUE format and an example, so operators can discover the feature via `./scripts/rolling-update.sh --help`. --- scripts/rolling-update.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 71f8367fe..7551f0bb1 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -55,6 +55,13 @@ Optional environment: RAFTADMIN_RPC_TIMEOUT_SECONDS RAFTADMIN_ALLOW_INSECURE + EXTRA_ENV + Whitespace-separated list of additional container environment variables to + forward to the remote docker run as `-e KEY=VALUE` flags. Format: + "KEY=VALUE [KEY=VALUE ...]" (e.g. "ELASTICKV_RAFT_DISPATCHER_LANES=1 ELASTICKV_PEBBLE_CACHE_MB=512"). + Each pair must be KEY=VALUE with a non-empty KEY; pairs themselves must not + contain whitespace. + Notes: - If RAFT_TO_REDIS_MAP is unset, it is derived automatically from NODES, RAFT_PORT, and REDIS_PORT. From 1a0cc2516438665da0e47b2ec570aef303bd23bf Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:04:08 +0900 Subject: [PATCH 05/12] fix(rolling-update): shell-escape EXTRA_ENV across SSH dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ssh(1) joins remaining argv into a single command string which the remote shell re-parses, so a whitespace-containing EXTRA_ENV (its documented format) was split on the remote side: the remote saw `EXTRA_ENV=FOO=1 BAR=2 bash -s`, which set EXTRA_ENV to only the first pair and turned the rest into additional `env` assignments. Escape EXTRA_ENV with `printf %q` before handing it to ssh so the remote shell reassembles the original string verbatim. Also escape S3_CREDENTIALS_FILE defensively — it is a user-supplied path that could plausibly contain spaces even if no caller does so today. --- scripts/rolling-update.sh | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 7551f0bb1..f9c031db3 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -399,6 +399,17 @@ update_one_node() { copy_raftadmin_to_remote "$node_id" "$ssh_target" + # ssh joins remaining arguments into a single command string which the remote + # shell re-parses, so values containing whitespace or shell metacharacters + # must be escaped before transport. EXTRA_ENV is documented as a + # whitespace-separated list of KEY=VALUE pairs and therefore always needs + # quoting; other forwarded variables are structurally whitespace-free today + # but we still escape the ones most likely to evolve (path-like values) for + # defense in depth. + local extra_env_escaped s3_credentials_file_escaped + extra_env_escaped="$(printf '%q' "${EXTRA_ENV:-}")" + s3_credentials_file_escaped="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")" + ssh "${SSH_BASE_OPTS[@]}" "$ssh_target" \ env \ IMAGE="$IMAGE" \ @@ -413,7 +424,7 @@ update_one_node() { S3_PORT="$S3_PORT" \ ENABLE_S3="$ENABLE_S3" \ S3_REGION="$S3_REGION" \ - S3_CREDENTIALS_FILE="$S3_CREDENTIALS_FILE" \ + S3_CREDENTIALS_FILE="$s3_credentials_file_escaped" \ S3_PATH_STYLE_ONLY="$S3_PATH_STYLE_ONLY" \ HEALTH_TIMEOUT_SECONDS="$HEALTH_TIMEOUT_SECONDS" \ LEADERSHIP_TRANSFER_TIMEOUT_SECONDS="$LEADERSHIP_TRANSFER_TIMEOUT_SECONDS" \ @@ -426,7 +437,7 @@ update_one_node() { ALL_NODE_HOSTS_CSV="$all_node_hosts_csv" \ RAFT_TO_REDIS_MAP="$RAFT_TO_REDIS_MAP" \ RAFT_TO_S3_MAP="$RAFT_TO_S3_MAP" \ - EXTRA_ENV="${EXTRA_ENV:-}" \ + EXTRA_ENV="$extra_env_escaped" \ bash -s <<'REMOTE' set -euo pipefail From 2fd1bf0ee325adc53ba368002e17d2a3208a9924 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:12:06 +0900 Subject: [PATCH 06/12] fix(rolling-update): validate EXTRA_ENV key is a POSIX identifier --- scripts/rolling-update.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index f9c031db3..0421cce3c 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -701,6 +701,11 @@ run_container() { echo "invalid EXTRA_ENV entry '$pair'; expected KEY=VALUE" >&2 exit 1 fi + local key="${pair%%=*}" + if [[ ! "$key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then + echo "invalid EXTRA_ENV key '$key' in entry '$pair'; key must match [A-Za-z_][A-Za-z0-9_]*" >&2 + exit 1 + fi extra_env_flags+=(-e "$pair") done fi From 5415d8fd49cb74dd34be45811cd78512b1120a84 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:21:13 +0900 Subject: [PATCH 07/12] perf(rolling-update): hoist SSH-escape out of per-node loop, escape path-vars Addresses gemini medium review on 2fd1bf0e: - EXTRA_ENV / S3_CREDENTIALS_FILE escaping did not change per node; moved from inside update_one_node (ran once per host) to file scope, computed once after ensure_remote_raftadmin_binaries. - For defense in depth, also escape IMAGE, DATA_DIR, SERVER_ENTRYPOINT -- path-like values forwarded over the SSH re-parse path. Structurally whitespace-free today, but cheap to harden against future deploy.env values containing metacharacters. - Non-path-like vars (CSV / numeric / boolean) left unquoted; adding printf %q there is inert but adds noise. Naming: *_Q suffix for the escaped forms to keep the env block readable. Smoke test: IMAGE=ghcr.io/bootjp/elastickv:latest -> unchanged DATA_DIR="/var/lib/my data/kv" -> remote sees [/var/lib/my data/kv] EXTRA_ENV="FOO=1 BAR=2" -> remote sees [FOO=1 BAR=2] --- scripts/rolling-update.sh | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 0421cce3c..0836ee85d 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -399,24 +399,13 @@ update_one_node() { copy_raftadmin_to_remote "$node_id" "$ssh_target" - # ssh joins remaining arguments into a single command string which the remote - # shell re-parses, so values containing whitespace or shell metacharacters - # must be escaped before transport. EXTRA_ENV is documented as a - # whitespace-separated list of KEY=VALUE pairs and therefore always needs - # quoting; other forwarded variables are structurally whitespace-free today - # but we still escape the ones most likely to evolve (path-like values) for - # defense in depth. - local extra_env_escaped s3_credentials_file_escaped - extra_env_escaped="$(printf '%q' "${EXTRA_ENV:-}")" - s3_credentials_file_escaped="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")" - ssh "${SSH_BASE_OPTS[@]}" "$ssh_target" \ env \ - IMAGE="$IMAGE" \ + IMAGE="$IMAGE_Q" \ RAFTADMIN_BIN_PATH="$RAFTADMIN_REMOTE_BIN" \ CONTAINER_NAME="$CONTAINER_NAME" \ - DATA_DIR="$DATA_DIR" \ - SERVER_ENTRYPOINT="$SERVER_ENTRYPOINT" \ + DATA_DIR="$DATA_DIR_Q" \ + SERVER_ENTRYPOINT="$SERVER_ENTRYPOINT_Q" \ RAFT_ENGINE="$RAFT_ENGINE" \ RAFT_PORT="$RAFT_PORT" \ REDIS_PORT="$REDIS_PORT" \ @@ -424,7 +413,7 @@ update_one_node() { S3_PORT="$S3_PORT" \ ENABLE_S3="$ENABLE_S3" \ S3_REGION="$S3_REGION" \ - S3_CREDENTIALS_FILE="$s3_credentials_file_escaped" \ + S3_CREDENTIALS_FILE="$S3_CREDENTIALS_FILE_Q" \ S3_PATH_STYLE_ONLY="$S3_PATH_STYLE_ONLY" \ HEALTH_TIMEOUT_SECONDS="$HEALTH_TIMEOUT_SECONDS" \ LEADERSHIP_TRANSFER_TIMEOUT_SECONDS="$LEADERSHIP_TRANSFER_TIMEOUT_SECONDS" \ @@ -437,7 +426,7 @@ update_one_node() { ALL_NODE_HOSTS_CSV="$all_node_hosts_csv" \ RAFT_TO_REDIS_MAP="$RAFT_TO_REDIS_MAP" \ RAFT_TO_S3_MAP="$RAFT_TO_S3_MAP" \ - EXTRA_ENV="$extra_env_escaped" \ + EXTRA_ENV="$EXTRA_ENV_Q" \ bash -s <<'REMOTE' set -euo pipefail @@ -853,6 +842,19 @@ fi ensure_local_raftadmin ensure_remote_raftadmin_binaries +# ssh joins remaining arguments into a single command string which the remote +# shell re-parses, so values containing whitespace or shell metacharacters +# must be escaped before transport. EXTRA_ENV is documented as a +# whitespace-separated list of KEY=VALUE pairs and therefore always needs +# quoting; other forwarded variables are structurally whitespace-free today +# but we still escape the path-like ones most likely to evolve for defense +# in depth. Escape once here since these don't change per node. +EXTRA_ENV_Q="$(printf '%q' "${EXTRA_ENV:-}")" +S3_CREDENTIALS_FILE_Q="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")" +IMAGE_Q="$(printf '%q' "$IMAGE")" +DATA_DIR_Q="$(printf '%q' "$DATA_DIR")" +SERVER_ENTRYPOINT_Q="$(printf '%q' "$SERVER_ENTRYPOINT")" + echo "[rolling-update] target image: $IMAGE" for node_id in "${ROLLING_NODE_IDS[@]}"; do update_one_node "$node_id" "$(node_host_by_id "$node_id")" "$(ssh_target_by_id "$node_id")" From 2014758cec3607e893f2c017d31534f7fd77dc23 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:28:28 +0900 Subject: [PATCH 08/12] fix(rolling-update): multi-line EXTRA_ENV + escape RAFTADMIN_REMOTE_BIN/CONTAINER_NAME Addresses two gemini medium review items on 5415d8fd: 1. Multi-line EXTRA_ENV values were silently truncated to the first line. `read -r -a <<< "$EXTRA_ENV"` consumes only up to the first newline of a here-string, so a deploy.env with EXTRA_ENV="FOO=1 BAR=2" would set FOO=1 and drop BAR=2 without error. Fix: normalise newlines to spaces before the read: extra_env_normalised="${EXTRA_ENV//$'\n'/ }" read -r -a extra_env_pairs <<< "${extra_env_normalised}" Smoke: three pairs spread across 2 lines now parse as 3 entries. 2. Extended `printf %q` defense-in-depth escaping to RAFTADMIN_REMOTE_BIN (path-like, may grow spaces) and CONTAINER_NAME (user-configurable). CSV / numeric / boolean vars remain unescaped. `bash -n scripts/rolling-update.sh` clean. --- scripts/rolling-update.sh | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 0836ee85d..829215886 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -402,8 +402,8 @@ update_one_node() { ssh "${SSH_BASE_OPTS[@]}" "$ssh_target" \ env \ IMAGE="$IMAGE_Q" \ - RAFTADMIN_BIN_PATH="$RAFTADMIN_REMOTE_BIN" \ - CONTAINER_NAME="$CONTAINER_NAME" \ + RAFTADMIN_BIN_PATH="$RAFTADMIN_REMOTE_BIN_Q" \ + CONTAINER_NAME="$CONTAINER_NAME_Q" \ DATA_DIR="$DATA_DIR_Q" \ SERVER_ENTRYPOINT="$SERVER_ENTRYPOINT_Q" \ RAFT_ENGINE="$RAFT_ENGINE" \ @@ -681,9 +681,14 @@ run_container() { # contain whitespace. local extra_env_flags=() if [[ -n "${EXTRA_ENV:-}" ]]; then - # Split on whitespace without triggering filename globbing. + # Split on whitespace without triggering filename globbing. Normalise + # newlines to spaces first so multi-line EXTRA_ENV values (common in + # long deploy.env overrides) are fully parsed — a here-string consumes + # only up to the first newline, so without this step subsequent lines + # would be silently dropped. local -a extra_env_pairs=() - read -r -a extra_env_pairs <<< "${EXTRA_ENV}" + local extra_env_normalised="${EXTRA_ENV//$'\n'/ }" + read -r -a extra_env_pairs <<< "${extra_env_normalised}" local pair for pair in "${extra_env_pairs[@]}"; do if [[ "$pair" != *=* || "$pair" == =* ]]; then @@ -854,6 +859,8 @@ S3_CREDENTIALS_FILE_Q="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")" IMAGE_Q="$(printf '%q' "$IMAGE")" DATA_DIR_Q="$(printf '%q' "$DATA_DIR")" SERVER_ENTRYPOINT_Q="$(printf '%q' "$SERVER_ENTRYPOINT")" +RAFTADMIN_REMOTE_BIN_Q="$(printf '%q' "$RAFTADMIN_REMOTE_BIN")" +CONTAINER_NAME_Q="$(printf '%q' "$CONTAINER_NAME")" echo "[rolling-update] target image: $IMAGE" for node_id in "${ROLLING_NODE_IDS[@]}"; do From 8ded4d1ec7c8d6aa0a46ebacac500b2ebaf77ec0 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:37:59 +0900 Subject: [PATCH 09/12] fix(rolling-update): normalise EXTRA_ENV newlines before printf %q Addresses gemini medium + codex P2 on 2014758c (same root cause). `printf '%q'` emits ANSI-C $'...' quoting for strings containing non- printable characters like newlines. The remote *login* shell re-parses the SSH command string before exec'ing `bash -s`. On Debian/Ubuntu where /bin/sh is dash, $'...' is not a special token -- the literal string is forwarded through, EXTRA_ENV arrives on the remote as `$'FOO=1\nBAR=2'`, and the KEY=VALUE validator in run_container rejects it, aborting the rollout. Fix: normalise newlines to spaces locally *before* printf %q, so the escape only emits plain backslash quoting which every POSIX shell understands. Confirms via smoke test: pre-fix: printf %q "FOO=1\nBAR=2" -> $'FOO=1\nBAR=2' (bash-only) post-fix: printf %q "FOO=1 BAR=2" -> FOO=1\ BAR=2 (portable) The existing in-run_container normalisation (also on newlines) is kept as defense in depth but is now unreachable via the rolling-update path. `bash -n scripts/rolling-update.sh` clean. --- scripts/rolling-update.sh | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 829215886..c8f3f0fbf 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -848,13 +848,22 @@ ensure_local_raftadmin ensure_remote_raftadmin_binaries # ssh joins remaining arguments into a single command string which the remote -# shell re-parses, so values containing whitespace or shell metacharacters -# must be escaped before transport. EXTRA_ENV is documented as a -# whitespace-separated list of KEY=VALUE pairs and therefore always needs -# quoting; other forwarded variables are structurally whitespace-free today -# but we still escape the path-like ones most likely to evolve for defense -# in depth. Escape once here since these don't change per node. -EXTRA_ENV_Q="$(printf '%q' "${EXTRA_ENV:-}")" +# login shell re-parses before `bash -s` is exec'd, so values containing +# whitespace or shell metacharacters must be escaped before transport. +# EXTRA_ENV is documented as a whitespace-separated list of KEY=VALUE pairs +# and therefore always needs quoting; other forwarded variables are +# structurally whitespace-free today but we still escape the path-like +# ones most likely to evolve for defense in depth. Escape once here since +# these don't change per node. +# +# EXTRA_ENV may legitimately span multiple lines in deploy.env; normalise +# newlines to spaces *before* `printf %q` so the escape emits plain +# backslash-quoting rather than ANSI-C $'...' quoting. Common remote login +# shells (/bin/sh -> dash on Debian/Ubuntu) don't grok $'...' and would +# pass it through as literal characters, breaking the `KEY=VALUE` +# validator in run_container. +EXTRA_ENV_NORMALISED="${EXTRA_ENV//$'\n'/ }" +EXTRA_ENV_Q="$(printf '%q' "${EXTRA_ENV_NORMALISED:-}")" S3_CREDENTIALS_FILE_Q="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")" IMAGE_Q="$(printf '%q' "$IMAGE")" DATA_DIR_Q="$(printf '%q' "$DATA_DIR")" From acc3ab1509a31d3ad01c8c583105fe6338911fd2 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:47:10 +0900 Subject: [PATCH 10/12] fix(rolling-update): EXTRA_ENV unset-safe + normalise tab/CR too Addresses gemini HIGH + codex P2 on 8ded4d1e. 1. Gemini HIGH -- `set -u` crash when EXTRA_ENV is unset. `${EXTRA_ENV//$'\n'/ }` dereferences $EXTRA_ENV, which under `set -u` aborts the script when the variable is unset (EXTRA_ENV is documented as optional in deploy.env). Fixed by going through `${EXTRA_ENV:-}` first. 2. Codex P2 -- `printf %q` still emits ANSI-C quoting for tabs and CR. Newline was only one of several non-printable characters that trigger the $'...' escape form. Tab and \r both do the same thing. Tabs can appear as legitimate separators; \r appears when deploy.env is edited on Windows. Normalise all three to spaces before %q. Smoke (all passing; see /tmp/pr591_smoke.sh): unset: [''] -- no crash CRLF+tab: [FOO=1\ \ BAR=2\ BAZ=3] -- dash-portable, no $'...' simple: [ELASTICKV_RAFT_DISPATCHER_LANES=1] glob-prone: [WILD=\*] -- not expanded multi-line: [FOO=1\ BAR=2\ BAZ=3] --- scripts/rolling-update.sh | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index c8f3f0fbf..47da36851 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -857,13 +857,17 @@ ensure_remote_raftadmin_binaries # these don't change per node. # # EXTRA_ENV may legitimately span multiple lines in deploy.env; normalise -# newlines to spaces *before* `printf %q` so the escape emits plain -# backslash-quoting rather than ANSI-C $'...' quoting. Common remote login -# shells (/bin/sh -> dash on Debian/Ubuntu) don't grok $'...' and would -# pass it through as literal characters, breaking the `KEY=VALUE` -# validator in run_container. -EXTRA_ENV_NORMALISED="${EXTRA_ENV//$'\n'/ }" -EXTRA_ENV_Q="$(printf '%q' "${EXTRA_ENV_NORMALISED:-}")" +# all non-space whitespace (newline, carriage return, tab) to spaces +# *before* `printf %q` so the escape emits plain backslash-quoting rather +# than ANSI-C $'...' quoting. Common remote login shells (/bin/sh -> dash +# on Debian/Ubuntu) don't grok $'...' and would pass it through as literal +# characters, breaking the `KEY=VALUE` validator in run_container. +# CR handling additionally covers deploy.env files edited on Windows. +# `${EXTRA_ENV:-}` is required because `set -u` is active and EXTRA_ENV +# may be unset (the variable is optional in deploy.env). +EXTRA_ENV_NORMALISED="${EXTRA_ENV:-}" +EXTRA_ENV_NORMALISED="${EXTRA_ENV_NORMALISED//[$'\t\r\n']/ }" +EXTRA_ENV_Q="$(printf '%q' "$EXTRA_ENV_NORMALISED")" S3_CREDENTIALS_FILE_Q="$(printf '%q' "${S3_CREDENTIALS_FILE:-}")" IMAGE_Q="$(printf '%q' "$IMAGE")" DATA_DIR_Q="$(printf '%q' "$DATA_DIR")" From 3a4e2bec68ab93f914665187dbb47a037f3a84fe Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:51:22 +0900 Subject: [PATCH 11/12] fix(rolling-update): escape RAFT_TO_REDIS_MAP / RAFT_TO_S3_MAP Addresses gemini medium on acc3ab15. Both CSV maps are derived (not directly user-supplied) and structurally contain only `=`, `:`, `,`, digits and identifier chars today -- none shell-special. `printf %q` is therefore a no-op under the current NODES format, but adding the escape aligns them with the other forwarded vars (IMAGE_Q, DATA_DIR_Q, etc.) so future evolution of the NODES schema can't silently corrupt the remote command line. --- scripts/rolling-update.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 47da36851..1a4b8ffff 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -424,8 +424,8 @@ update_one_node() { NODE_HOST="$node_host" \ ALL_NODE_IDS_CSV="$all_node_ids_csv" \ ALL_NODE_HOSTS_CSV="$all_node_hosts_csv" \ - RAFT_TO_REDIS_MAP="$RAFT_TO_REDIS_MAP" \ - RAFT_TO_S3_MAP="$RAFT_TO_S3_MAP" \ + RAFT_TO_REDIS_MAP="$RAFT_TO_REDIS_MAP_Q" \ + RAFT_TO_S3_MAP="$RAFT_TO_S3_MAP_Q" \ EXTRA_ENV="$EXTRA_ENV_Q" \ bash -s <<'REMOTE' set -euo pipefail @@ -874,6 +874,8 @@ DATA_DIR_Q="$(printf '%q' "$DATA_DIR")" SERVER_ENTRYPOINT_Q="$(printf '%q' "$SERVER_ENTRYPOINT")" RAFTADMIN_REMOTE_BIN_Q="$(printf '%q' "$RAFTADMIN_REMOTE_BIN")" CONTAINER_NAME_Q="$(printf '%q' "$CONTAINER_NAME")" +RAFT_TO_REDIS_MAP_Q="$(printf '%q' "$RAFT_TO_REDIS_MAP")" +RAFT_TO_S3_MAP_Q="$(printf '%q' "$RAFT_TO_S3_MAP")" echo "[rolling-update] target image: $IMAGE" for node_id in "${ROLLING_NODE_IDS[@]}"; do From e871564b8eb4d352337e0775b4051736bc93a410 Mon Sep 17 00:00:00 2001 From: "Yoshiaki Ueda (bootjp)" Date: Thu, 23 Apr 2026 19:56:39 +0900 Subject: [PATCH 12/12] fix(rolling-update): pin IFS when splitting EXTRA_ENV Addresses gemini medium on 3a4e2bec. `read -r -a` inherits the ambient IFS. Default is already space/tab/ newline, but making the splitting deterministic against any future IFS mutation elsewhere in the script (or sourced file) is cheap insurance. Smoke-tested: with IFS=":" in the outer scope, the pinned IFS=$' \t\n' still splits "FOO=1 BAR=2 BAZ=3" into three entries. --- scripts/rolling-update.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/rolling-update.sh b/scripts/rolling-update.sh index 1a4b8ffff..d84c16f6d 100755 --- a/scripts/rolling-update.sh +++ b/scripts/rolling-update.sh @@ -688,7 +688,10 @@ run_container() { # would be silently dropped. local -a extra_env_pairs=() local extra_env_normalised="${EXTRA_ENV//$'\n'/ }" - read -r -a extra_env_pairs <<< "${extra_env_normalised}" + # Explicit IFS so splitting is immune to any earlier mutation. Default + # whitespace is already space/tab/newline; pinning it here makes the + # behaviour self-documenting. + IFS=$' \t\n' read -r -a extra_env_pairs <<< "${extra_env_normalised}" local pair for pair in "${extra_env_pairs[@]}"; do if [[ "$pair" != *=* || "$pair" == =* ]]; then