Skip to content

Commit 3fe2c45

Browse files
committed
scripts: localize all 9 ADMIN_* in build_admin_flags
Two gemini medium findings on PR #678 caught that the defense-in-depth localization I added was incomplete: ADMIN_SESSION_SIGNING_KEY_FILE et al got `${VAR:-}` defaults into locals at the top of the helper, but ADMIN_ENABLED and the two ADMIN_ALLOW_* booleans were still accessed directly from the calling environment further down in the function. The gap defeated the comment's own claim. If a future refactor ever drops one of those three booleans from the env forwarding, `set -u` would crash on `${ADMIN_ENABLED}` (and the code path below would silently fall through to defaults for the two ALLOW_* flags, masking the misconfiguration). The point of the local-with-default pattern is that every ADMIN_* reference goes through one place where the safety net is guaranteed. Localized all nine into `enabled`, `signing_key`, `full_keys`, `read_only_keys`, `previous_key`, `admin_listen`, `tls_cert`, `tls_key`, `allow_plaintext`, `insecure_cookie`. The two ALLOW_* check sites at the bottom now read the locals instead of re-fetching the globals — same value, but consistent with the rest of the helper and the comment's contract. No behaviour change for any valid input. Smoke-tested both boolean validators (`ADMIN_ENABLED=invalid` and `ADMIN_ALLOW_PLAINTEXT_NON_LOOPBACK=yes`) — local script-level errors still fire with the targeted message before reaching update_one_node.
1 parent 14ec553 commit 3fe2c45

1 file changed

Lines changed: 10 additions & 4 deletions

File tree

scripts/rolling-update.sh

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -872,8 +872,12 @@ build_admin_flags() {
872872
# update_one_node. If a future refactor ever drops one of the
873873
# forwarded variables, the operator gets the targeted "ADMIN_*
874874
# required" error below instead of an opaque "unbound variable"
875-
# crash with no hint at which variable.
876-
if [[ "${ADMIN_ENABLED:-false}" != "true" ]]; then
875+
# crash with no hint at which variable. All nine ADMIN_* values
876+
# are read into locals once at the top so the rest of the helper
877+
# cannot accidentally re-fetch a global and bypass the safety net
878+
# (gemini medium on PR #678 caught the original three-boolean gap).
879+
local enabled="${ADMIN_ENABLED:-false}"
880+
if [[ "$enabled" != "true" ]]; then
877881
return 0
878882
fi
879883
@@ -884,6 +888,8 @@ build_admin_flags() {
884888
local admin_listen="${ADMIN_ADDRESS:-}"
885889
local tls_cert="${ADMIN_TLS_CERT_FILE:-}"
886890
local tls_key="${ADMIN_TLS_KEY_FILE:-}"
891+
local allow_plaintext="${ADMIN_ALLOW_PLAINTEXT_NON_LOOPBACK:-false}"
892+
local insecure_cookie="${ADMIN_ALLOW_INSECURE_DEV_COOKIE:-false}"
887893
888894
if [[ -z "$signing_key" ]]; then
889895
echo "ADMIN_ENABLED=true requires ADMIN_SESSION_SIGNING_KEY_FILE; aborting" >&2
@@ -944,10 +950,10 @@ build_admin_flags() {
944950
_volumes+=(-v "${tls_key}:${tls_key}:ro")
945951
fi
946952
947-
if [[ "${ADMIN_ALLOW_PLAINTEXT_NON_LOOPBACK:-false}" == "true" ]]; then
953+
if [[ "$allow_plaintext" == "true" ]]; then
948954
_flags+=(--adminAllowPlaintextNonLoopback)
949955
fi
950-
if [[ "${ADMIN_ALLOW_INSECURE_DEV_COOKIE:-false}" == "true" ]]; then
956+
if [[ "$insecure_cookie" == "true" ]]; then
951957
_flags+=(--adminAllowInsecureDevCookie)
952958
fi
953959
}

0 commit comments

Comments
 (0)