diff --git a/scripts/probe-member-credentials.sh b/scripts/probe-member-credentials.sh index b92971d..f65a8a5 100755 --- a/scripts/probe-member-credentials.sh +++ b/scripts/probe-member-credentials.sh @@ -64,8 +64,86 @@ # The two outputs side by side are the finding: any name whose hash matches between them is a # credential the member holds in full. # +# 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. +parse_policy_fields() { + if command -v jq >/dev/null 2>&1; then + _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | jq -r ' + (.present | tostring), + (.policy // ""), + (.knownCount // 0 | tostring), + (.allowedCount // 0 | tostring), + (.blockedCount // 0 | tostring), + (.known[]? // empty)')" + _PARSE_STATUS=$? + _PARSER_NAME="jq" + else + _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | python3 - <<'PY' +import json, sys +data = json.load(sys.stdin) +print(str(data.get("present"))) +print(data.get("policy") or "") +print(data.get("knownCount") if data.get("knownCount") is not None else 0) +print(data.get("allowedCount") if data.get("allowedCount") is not None else 0) +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 + return 4 + fi + + # A herestring adds a newline, so mapfile would turn an empty parser result into one empty field. + # Keep that case separate so the refusal reports what the parser actually returned: zero fields. + if [ -z "$_FIELDS_RAW" ]; then + _FIELDS=() + else + mapfile -t _FIELDS <<< "$_FIELDS_RAW" + fi + + # 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 fixed-field read in main() expects, + # whatever the reason: a producer that printed nothing, malformed JSON that jq/python3 still + # accepted, or a schema change upstream. main()'s slice (`_FIELDS[@]:5`) does not fire + # `set -u` on an unset OR a short array, and every fixed-field read there 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 + return 5 + fi +} + +main() { set -uo pipefail -# `pipefail` is not what catches the parser failure handled below (fleetd #500): in +# `pipefail` is not what catches the parser failure handled in parse_policy_fields() above (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 @@ -142,74 +220,10 @@ EOF exit 1 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 - _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | jq -r ' - (.present | tostring), - (.policy // ""), - (.knownCount // 0 | tostring), - (.allowedCount // 0 | tostring), - (.blockedCount // 0 | tostring), - (.known[]? // empty)')" - _PARSE_STATUS=$? - _PARSER_NAME="jq" -else - _FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | python3 - <<'PY' -import json, sys -data = json.load(sys.stdin) -print(str(data.get("present"))) -print(data.get("policy") or "") -print(data.get("knownCount") if data.get("knownCount") is not None else 0) -print(data.get("allowedCount") if data.get("allowedCount") is not None else 0) -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 +# Parse the policy in one pass and refuse on any of the three failure causes. The decision, the +# three refusals and the reasoning behind each live in parse_policy_fields() above — kept there +# with the code rather than here, so the explanation cannot drift away from what it explains. +parse_policy_fields || exit $? PRESENT="${_FIELDS[0]:-null}" POLICY_MODE="${_FIELDS[1]:-}" @@ -317,3 +331,8 @@ How to read this: hardcoded list did. If the daemon's policy changes, the next run of this script reflects it with no edit to this file. EOF +} + +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + main "$@" +fi diff --git a/scripts/test-probe-member-credentials.sh b/scripts/test-probe-member-credentials.sh new file mode 100755 index 0000000..613e1c8 --- /dev/null +++ b/scripts/test-probe-member-credentials.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# Self-contained checks for the policy parsing guards in probe-member-credentials.sh. + +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PROBE="$ROOT/scripts/probe-member-credentials.sh" +TMP="$(mktemp -d "$ROOT/.probe-member-credentials-test.XXXXXX")" +trap 'rm -rf "$TMP"' EXIT + +# The SOURCED guard exposes this pure parser without contacting POLICY_URL. +source "$PROBE" + +fail() { + printf 'FAIL: %s\n' "$*" >&2 + return 1 +} + +assert_equals() { + local expected="$1" actual="$2" description="$3" + [ "$expected" = "$actual" ] || fail "$description: expected $expected, got $actual" +} + +assert_contains() { + local needle="$1" text="$2" description="$3" + printf '%s' "$text" | grep -qF "$needle" || fail "$description: missing $needle" +} + +make_jq() { + local body="$1" + mkdir -p "$TMP/bin" + printf '%s\n' '#!/usr/bin/env bash' "$body" > "$TMP/bin/jq" + chmod +x "$TMP/bin/jq" +} + +run_parser() { + local output rc=0 + POLICY_JSON="$(< "$TMP/policy.json")" + POLICY_URL="fixture://member-credentials" + output="$(PATH="$TMP/bin:$PATH" parse_policy_fields 2>&1)" || rc=$? + PARSER_OUTPUT="$output" + PARSER_RC="$rc" +} + +test_bash_older_than_four_refuses() { + local output rc=0 version + version="$(/bin/bash -c 'printf %s "$BASH_VERSION"')" + output="$(/bin/bash "$PROBE" 2>&1)" || rc=$? + assert_equals 3 "$rc" "bash 3 refusal status" + assert_contains 'This shell is bash' "$output" "bash 3 refusal" + assert_contains "$version" "$output" "bash 3 refusal version" +} + +test_parser_non_zero_refuses() { + make_jq 'exit 17' + run_parser + assert_equals 4 "$PARSER_RC" "parser failure status" + assert_contains 'jq exited non-zero (status 17)' "$PARSER_OUTPUT" "parser failure message" +} + +test_short_parser_output_refuses() { + make_jq "printf '%s\\n' true enforce 3 2" + run_parser + assert_equals 5 "$PARSER_RC" "short parser output status" + assert_contains 'policy parser (jq) returned 4 field(s)' "$PARSER_OUTPUT" "short parser output count" +} + +test_empty_parser_output_reports_zero_fields() { + make_jq ':' + run_parser + assert_equals 5 "$PARSER_RC" "empty parser output status" + assert_contains 'policy parser (jq) returned 0 field(s)' "$PARSER_OUTPUT" "empty parser output count" +} + +test_well_formed_policy_prints_name_table() { + local output rc=0 + make_jq "cat '$TMP/policy.fields'" + # Shell functions cannot be passed in an environment assignment. Run the executable through bash. + output="$(BRIDGED_MEMBER=1 FIXTURE="$TMP/policy.json" PROBE="$PROBE" PATH="$TMP/bin:$PATH" bash -c ' + curl() { cat "$FIXTURE"; } + export -f curl + exec "$PROBE" + ' 2>&1)" || rc=$? + assert_equals 0 "$rc" "well-formed policy status" + assert_contains 'ALPHA_TOKEN' "$output" "name table" + assert_contains 'BETA_TOKEN' "$output" "name table" + assert_contains 'GAMMA_TOKEN' "$output" "name table" +} + +cat > "$TMP/policy.json" <<'JSON' +{"present":true,"policy":"enforce","knownCount":3,"allowedCount":2,"blockedCount":1,"known":["ALPHA_TOKEN","BETA_TOKEN","GAMMA_TOKEN"]} +JSON +cat > "$TMP/policy.fields" <<'FIELDS' +true +enforce +3 +2 +1 +ALPHA_TOKEN +BETA_TOKEN +GAMMA_TOKEN +FIELDS + +test_bash_older_than_four_refuses +test_parser_non_zero_refuses +test_short_parser_output_refuses +test_empty_parser_output_reports_zero_fields +test_well_formed_policy_prints_name_table +printf 'PASS: probe member credentials guards\n'