From 28b45d97e5dec43ebbc6e5f3885c75d21463d918 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 3 Oct 2026 22:06:54 +0200 Subject: [PATCH 1/2] fleetd #638: mask userinfo in the daemon verdict line before it prints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply the userinfo-only rewrite (sed -E 's#://[^@]*@#://@#g') to every path that prints config-edit.sh's $VERDICT_LINE: restore_and_confirm's two prints, report_outcome's shared local copy (covering its clean/needs-restart/ refused branches), and check_mode's "last verdict in log" line via last_verdict_line. Deliberately not routed through redact() — that function's key:value masking does not match this line's prose, and the rest of the line (e.g. the pattern quoted in a parse-failure refusal) is the detail an operator needs to fix the refusal. No current refusal message echoes a URI, token or password, so this is a guard against a future validator doing so, not a fix for an observed leak. --- scripts/config-edit.sh | 16 +++++++++---- scripts/test-config-edit.sh | 45 +++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/scripts/config-edit.sh b/scripts/config-edit.sh index 9a25283f..24132a32 100755 --- a/scripts/config-edit.sh +++ b/scripts/config-edit.sh @@ -580,6 +580,14 @@ install_candidate() { # -------------------------------------------------------------------------------- the report path # +# Masks basic-auth userinfo (scheme://user:pass@host) in a daemon verdict line before it reaches +# the terminal. Not routed through redact(): that function's key:value masking does not match this +# line's prose, and masking anything beyond the userinfo would remove the detail an operator needs +# to diagnose a refusal. +mask_verdict_userinfo() { + printf '%s\n' "$1" | sed -E 's#://[^@]*@#://@#g' +} + # Prints the literal command the operator (or a test) can run to restore the backup by hand — the # absolute path to THIS script plus the overrides actually in force, so it works from any cwd. restore_command_line() { @@ -599,8 +607,8 @@ restore_and_confirm() { ok "restored from $backup" if wait_for_verdict "$LOG" "$mark2" "$WAIT_SECONDS"; then case "$VERDICT_KIND" in - refused) warn "the RESTORE was also refused by the daemon: $VERDICT_LINE" ;; - *) ok "restore confirmed: $VERDICT_LINE" ;; + refused) warn "the RESTORE was also refused by the daemon: $(mask_verdict_userinfo "$VERDICT_LINE")" ;; + *) ok "restore confirmed: $(mask_verdict_userinfo "$VERDICT_LINE")" ;; esac else warn "the restore is on disk, but no confirming verdict line appeared within ${WAIT_SECONDS}s" @@ -616,7 +624,7 @@ report_outcome() { say "waiting for the daemon's verdict (up to ${WAIT_SECONDS}s)" if wait_for_verdict "$LOG" "$mark" "$WAIT_SECONDS"; then - kind="$VERDICT_KIND"; line="$VERDICT_LINE" + kind="$VERDICT_KIND"; line="$(mask_verdict_userinfo "$VERDICT_LINE")" else kind="none" fi @@ -673,7 +681,7 @@ check_mode() { local verdict verdict="$(last_verdict_line "$LOG")" if [ -n "$verdict" ]; then - ok "last verdict in log: $verdict" + ok "last verdict in log: $(mask_verdict_userinfo "$verdict")" else warn "no reload verdict line found in $LOG" fi diff --git a/scripts/test-config-edit.sh b/scripts/test-config-edit.sh index 56695d0f..326f6648 100755 --- a/scripts/test-config-edit.sh +++ b/scripts/test-config-edit.sh @@ -629,6 +629,47 @@ test_refusal_shape_from_parse_failure_wording_is_recognised() { assert_equals 4 "$RUN_RC" "the parse-failure refusal shape must also exit 4, not be read as silence" } +# A verdict line carrying a credentialed URI has its userinfo masked, with a positive control +# proving the rest of the line still reaches the output unchanged. +test_verdict_userinfo_is_masked_with_positive_control() { + local dir + dir="$(new_fixture)" + + start_run "$dir" 5 --set '.profiles.sonnet.weight=4' + sleep 1 + printf 'config reload from %s refused, keeping the running config: refusing to start: malformed pattern — profiles.local.errorPattern ("amqp://user:hunter2@host/vhost"): Unclosed character class near index 8\n' \ + "$dir/fleetd.yaml" >> "$dir/fleetd.out" + collect_run "$dir" + + assert_equals 4 "$RUN_RC" "refusal-with-userinfo exit code" + assert_not_contains "user:hunter2" "$RUN_OUTPUT" "the userinfo must never reach the output" + assert_contains "amqp://@host/vhost" "$RUN_OUTPUT" \ + "the userinfo must be MASKED, not deleted — the rest of the quoted value must survive" + # Positive control: the diagnostic prose on both sides of the userinfo must still reach the + # output. Without this, a mutant that drops the whole verdict line would pass identically. + assert_contains "malformed pattern" "$RUN_OUTPUT" "prose BEFORE the userinfo must still reach the output" + assert_contains "Unclosed character class near index 8" "$RUN_OUTPUT" \ + "prose AFTER the userinfo must still reach the output" +} + +# An ordinary refusal line quotes the offending pattern, not a credential, and must survive byte +# for byte: the rewrite is scoped to userinfo only, and the quoted pattern is the detail an +# operator needs to fix the refusal. +test_ordinary_refusal_line_passes_through_unchanged() { + local dir real_line + dir="$(new_fixture)" + real_line="config reload from $dir/fleetd.yaml refused, keeping the running config: refusing to start: malformed pattern — profiles.local.errorPattern (\"[unclosed\"): Unclosed character class near index 8" + + start_run "$dir" 5 --set '.profiles.sonnet.weight=4' + sleep 1 + printf '%s\n' "$real_line" >> "$dir/fleetd.out" + collect_run "$dir" + + assert_equals 4 "$RUN_RC" "ordinary refusal exit code" + assert_contains "$real_line" "$RUN_OUTPUT" \ + "an ordinary refusal with no userinfo must pass through byte for byte, unchanged" +} + # --set runs yq over the whole candidate. It warns when that changes more lines than the requested # pairs, but a simple file with only the intended changed line must stay quiet. new_fixture_reformat_sensitive() { @@ -720,6 +761,10 @@ echo "== extra: --check is read-only and always exits 0 ==" test_check_is_read_only_and_exits_zero echo "== extra: the parse-failure refusal shape is also recognised ==" test_refusal_shape_from_parse_failure_wording_is_recognised +echo "== verdict-redaction criteria 2+3: verdict userinfo is masked, rest of line survives ==" +test_verdict_userinfo_is_masked_with_positive_control +echo "== verdict-redaction criterion 4: an ordinary refusal passes through unchanged ==" +test_ordinary_refusal_line_passes_through_unchanged echo "== acceptance criterion 17: --set warns about yq formatting churn ==" test_set_warns_when_yq_reformats_extra_lines echo "== acceptance criterion 18: --set stays quiet without formatting churn ==" From 5f5d16fbd47b47d6cdfd09d53abf8644991eef3e Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 3 Oct 2026 22:34:02 +0200 Subject: [PATCH 2/2] fleetd #638: stop the verdict userinfo mask from crossing / or whitespace mask_verdict_userinfo's character class [^@]* crossed a '/' or a space, so a verdict line with a URI that has no userinfo plus a later @ (e.g. an email address in diagnostic prose) had everything between them destroyed. Restrict the class to [^@/[:space:]]* so the match stops at the end of the URI. Adds the uncovered-direction test: a URI with no userinfo plus a later @ in the same line must pass through byte for byte. Also drops the design-rationale sentence from the helper's comment (now in the PR description). --- scripts/config-edit.sh | 6 ++---- scripts/test-config-edit.sh | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/scripts/config-edit.sh b/scripts/config-edit.sh index 24132a32..3f1e61fc 100755 --- a/scripts/config-edit.sh +++ b/scripts/config-edit.sh @@ -581,11 +581,9 @@ install_candidate() { # -------------------------------------------------------------------------------- the report path # # Masks basic-auth userinfo (scheme://user:pass@host) in a daemon verdict line before it reaches -# the terminal. Not routed through redact(): that function's key:value masking does not match this -# line's prose, and masking anything beyond the userinfo would remove the detail an operator needs -# to diagnose a refusal. +# the terminal. mask_verdict_userinfo() { - printf '%s\n' "$1" | sed -E 's#://[^@]*@#://@#g' + printf '%s\n' "$1" | sed -E 's#://[^@/[:space:]]*@#://@#g' } # Prints the literal command the operator (or a test) can run to restore the backup by hand — the diff --git a/scripts/test-config-edit.sh b/scripts/test-config-edit.sh index 326f6648..6b957f83 100755 --- a/scripts/test-config-edit.sh +++ b/scripts/test-config-edit.sh @@ -670,6 +670,24 @@ test_ordinary_refusal_line_passes_through_unchanged() { "an ordinary refusal with no userinfo must pass through byte for byte, unchanged" } +# A verdict line can hold a URI with NO userinfo and a later, unrelated @ further on in the same +# line (an email address in diagnostic prose, for example). The rewrite must stop at the end of +# the URI and must not treat the later @ as a second userinfo delimiter. +test_uri_without_userinfo_survives_a_later_at_sign() { + local dir real_line + dir="$(new_fixture)" + real_line="config reload from $dir/fleetd.yaml refused, keeping the running config: broker.uri amqp://broker.local/vhost unreachable, contact ops@example.com" + + start_run "$dir" 5 --set '.profiles.sonnet.weight=4' + sleep 1 + printf '%s\n' "$real_line" >> "$dir/fleetd.out" + collect_run "$dir" + + assert_equals 4 "$RUN_RC" "no-userinfo-with-later-at-sign exit code" + assert_contains "$real_line" "$RUN_OUTPUT" \ + "a URI with no userinfo plus a later @ in the same line must pass through byte for byte" +} + # --set runs yq over the whole candidate. It warns when that changes more lines than the requested # pairs, but a simple file with only the intended changed line must stay quiet. new_fixture_reformat_sensitive() { @@ -765,6 +783,8 @@ echo "== verdict-redaction criteria 2+3: verdict userinfo is masked, rest of lin test_verdict_userinfo_is_masked_with_positive_control echo "== verdict-redaction criterion 4: an ordinary refusal passes through unchanged ==" test_ordinary_refusal_line_passes_through_unchanged +echo "== fleetd #638: a URI with no userinfo survives a later @ in the same line ==" +test_uri_without_userinfo_survives_a_later_at_sign echo "== acceptance criterion 17: --set warns about yq formatting churn ==" test_set_warns_when_yq_reformats_extra_lines echo "== acceptance criterion 18: --set stays quiet without formatting churn =="