From d59ece6dece66399625fabb00b7c47e08d36b44c Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 10:58:16 +0700 Subject: [PATCH] fleetd #500: stop a wrong-interpreter or failed-parse reading a policy as empty probe-member-credentials.sh used mapfile < <(producer) to parse the fetched policy. That hides a producer failure three ways: mapfile is bash 4+ and missing on macOS's /bin/bash 3.2, a process substitution's exit status is never propagated to mapfile, and the downstream reads (":-" defaults and a slice) never fire set -u on a short or unset array. All three converge on the same "0 known names" refusal, which blames the policy for a failure that is actually the interpreter or the parser. Three distinct guards, each closing one cause with its own message: - a BASH_VERSINFO gate at the top refuses outright on bash < 4 (exit 3) - the parser's output is captured via command substitution instead of mapfile < <(...), so a non-zero jq/python3 exit is caught at the call while the fact still exists (exit 4) - an arity check before the field slice refuses a parse that exits 0 but returns fewer than 5 fields (exit 5) The existing "0 known names" guard is now honest: by the time it fires, the three causes above are already ruled out, so it really does mean the policy has 0 known names. --- scripts/probe-member-credentials.sh | 90 +++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 12 deletions(-) diff --git a/scripts/probe-member-credentials.sh b/scripts/probe-member-credentials.sh index 11f26e5..b92971d 100755 --- a/scripts/probe-member-credentials.sh +++ b/scripts/probe-member-credentials.sh @@ -65,6 +65,27 @@ # credential the member holds in full. # set -uo pipefail +# `pipefail` is not what catches the parser failure handled below (fleetd #500): in +# `printf '%s' "$POLICY_JSON" | jq -r '...'`, jq is the LAST element of the pipe, so the pipeline's +# own exit status is already jq's status, with or without pipefail. It is kept as insurance for if +# a post-processing stage is ever appended after the parser (e.g. `| tail -n +2`) — at that point +# the parser would sit upstream and pipefail becomes the only thing that still reports its status. + +# --- refuse on an interpreter that cannot run this script (fleetd #500) ------------------------- +# +# mapfile, used below to parse the policy response, was added in bash 4.0. macOS ships bash 3.2.57 +# at /bin/bash, which predates it. This script's own `set -uo pipefail` does not catch a missing +# mapfile: the builtin just fails with "command not found" on stderr, and every line below that +# reads the array it would have filled uses a `:-` default or a slice, neither of which `set -u` +# catches on an unset array. Left unguarded, that chain ends in the "0 known names" refusal further +# down — a claim about the POLICY, for a failure that is actually about the INTERPRETER. So the +# interpreter is checked once, explicitly, before it is asked to do anything mapfile depends on. +if (( ${BASH_VERSINFO[0]} < 4 )); then + echo "refusing to run: this script uses mapfile, which needs bash 4 or newer. This shell is bash" \ + "${BASH_VERSION:-}. Re-run it under a newer bash, for example:" \ + "\"\$(command -v bash)\" \"$0\"" "$@" >&2 + exit 3 +fi FLEETD_HOST="${FLEETD_HOST:-http://127.0.0.1:8765}" POLICY_URL="${FLEETD_HOST%/}/member-credentials" @@ -124,16 +145,32 @@ fi # One parse pass: line 1 = present (true/false/null), line 2 = policy mode (possibly blank), # lines 3-5 = knownCount/allowedCount/blockedCount, remaining lines = the known[] names. A single # pass avoids re-parsing (and re-risking a truthiness bug) five separate times. +# +# This used to feed the parser straight into `mapfile -t _FIELDS < <(producer)`. That form cannot +# see the producer fail: `<` `<(...)` is a process substitution, not a pipeline, so `set -o +# pipefail` does not reach inside it, and mapfile's own exit status reports whether the BUILTIN +# ran, not whether the command substituted into it succeeded — a failing jq or python3 there still +# leaves mapfile at rc=0 with an empty array, read as a parse that genuinely found nothing (fleetd +# #500). Capturing the parser's output with command substitution first, and checking ITS exit +# status, reports the producer's real failure while the fact still exists — before it is handed to +# mapfile at all. +# +# mapfile then reads from that captured string with `<<<` (a herestring), not `< <(...)`: `<<<` +# materialises the whole string in memory first, where `< <(...)` would stream it. That only +# matters for a large producer; this one is a short credential-name policy response, so the +# tradeoff is irrelevant here — noted because it would not be for every producer. if command -v jq >/dev/null 2>&1; then - mapfile -t _FIELDS < <(printf '%s' "$POLICY_JSON" | jq -r ' + _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | jq -r ' (.present | tostring), (.policy // ""), (.knownCount // 0 | tostring), (.allowedCount // 0 | tostring), (.blockedCount // 0 | tostring), - (.known[]? // empty)') + (.known[]? // empty)')" + _PARSE_STATUS=$? + _PARSER_NAME="jq" else - mapfile -t _FIELDS < <(printf '%s' "$POLICY_JSON" | python3 - <<'PY' + _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | python3 - <<'PY' import json, sys data = json.load(sys.stdin) print(str(data.get("present"))) @@ -144,7 +181,34 @@ print(data.get("blockedCount") if data.get("blockedCount") is not None else 0) for n in (data.get("known") or []): print(n) PY - ) + )" + _PARSE_STATUS=$? + _PARSER_NAME="python3" +fi + +if [ "$_PARSE_STATUS" -ne 0 ]; then + echo "refusing to run: could not parse the policy fetched from $POLICY_URL — $_PARSER_NAME exited" \ + "non-zero (status $_PARSE_STATUS). That is a parser failure, not a claim about the policy" \ + "itself; the policy response has not been read." >&2 + exit 4 +fi + +mapfile -t _FIELDS <<< "$_FIELDS_RAW" + +# Arity check — the CORRECTNESS fix (fleetd #500). A parser that exits 0 can still return fewer +# than the 5 fixed fields (present, policy mode, 3 counts) that every line below this expects, +# whatever the reason: a producer that printed nothing, malformed JSON that jq/python3 still +# accepted, or a schema change upstream. The slice just below this (`_FIELDS[@]:5`) does not fire +# `set -u` on an unset OR a short array, and every fixed-field read above used a `:-` default, so +# without this check a short `_FIELDS` reaches the "0 known names" guard further down with the +# same look as a policy that genuinely has 0 names. Check the count here, at the one point the +# fact is still present, before the slice consumes it. +if (( ${#_FIELDS[@]} < 5 )); then + echo "refusing to run: the policy parser ($_PARSER_NAME) returned ${#_FIELDS[@]} field(s); at" \ + "least 5 are required (present, policy mode, knownCount, allowedCount, blockedCount). The" \ + "parse ran but its shape is wrong — this is not a claim about how many names the policy" \ + "knows." >&2 + exit 5 fi PRESENT="${_FIELDS[0]:-null}" @@ -164,19 +228,21 @@ case "$KNOWN_COUNT_REPORTED" in ;; esac -# --- guard the denominator explicitly — never proceed on a zero/short count --------------------- +# --- guard the denominator explicitly — never proceed on a zero count --------------------------- # -# This is the exact trap named in the ticket: an empty (or truncated) NAMES array passes every -# subsequent "is it set" check vacuously and prints a table that LOOKS complete. So this is checked -# before anything else runs, with a message that says why, not just that it failed. +# This is the exact trap named in the ticket: an empty NAMES array passes every subsequent "is it +# set" check vacuously and prints a table that LOOKS complete. By this point the interpreter gate, +# the parser-exit-status check, and the arity check above have already ruled out "the interpreter +# couldn't run mapfile", "the parser failed", and "the parser returned the wrong shape" — so a zero +# count reaching here really does mean the policy itself reports 0 known names, not a swallowed +# failure upstream. That is still checked before anything else runs, with a message that says so. if [ "${#NAMES[@]}" -eq 0 ] || [ "$KNOWN_COUNT_REPORTED" -eq 0 ]; then cat >&2 <