fleetd #638: mask userinfo in the daemon verdict line before it prints

Apply the userinfo-only rewrite (sed -E 's#://[^@]*@#://<redacted>@#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.

fleetd/src/test/java/dev/ltms/fleet/FleetdAssemblyTimingDefaultsTest.java

@@ -1,190 +0,0 @@
-package dev.ltms.fleet;
-
-import dev.ltms.fleet.config.ConfigRef;
-import dev.ltms.fleet.config.FleetConfig;
-import dev.ltms.fleet.guard.SubscriptionGuard;
-import dev.ltms.fleet.herdr.FakeHerdr;
-import dev.ltms.fleet.herdr.HerdrClient;
-import dev.ltms.fleet.inject.Injector;
-import dev.ltms.fleet.inject.StatusPoller;
-import dev.ltms.fleet.msg.LeadChannelHandle;
-import dev.ltms.fleet.msg.LeadCoordLoop;
-import dev.ltms.fleet.msg.LeadMessage;
-import dev.ltms.fleet.msg.ReplyInbox;
-import dev.ltms.fleet.msg.ReplyPushLoop;
-import io.javalin.Javalin;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.io.TempDir;
-
-import java.lang.reflect.Field;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.function.LongSupplier;
-
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-
-/**
- * Asserts that the assembled loops use the production reminder, coordination, and delivery timing
- * defaults when no {@code primary:} block configures the reply-push values.
- */
-class FleetdAssemblyTimingDefaultsTest {
-
-    private static final class FakeLeadChannel implements LeadChannelHandle {
-        @Override
-        public void publish(String toCoordId, LeadMessage message) {
-        }
-
-        @Override
-        public List<LeadMessage> peek() {
-            return List.of();
-        }
-
-        @Override
-        public void ack(String msgId) {
-        }
-
-        @Override
-        public String selfCoordId() {
-            return "test-lead";
-        }
-
-        @Override
-        public boolean heldDurable() {
-            return true;
-        }
-
-        @Override
-        public MailboxState inspect(String coordId) {
-            return MailboxState.unknown(coordId);
-        }
-
-        @Override
-        public void close() {
-        }
-    }
-
-    private static final class TestResourcePorts implements ResourcePorts {
-        final FakeHerdr herdr = new FakeHerdr();
-        Runnable shutdownHook;
-
-        @Override
-        public Map<String, String> environment() {
-            return Map.of();
-        }
-
-        @Override
-        public HerdrClient connectHerdr(Path socketPath) {
-            return herdr;
-        }
-
-        @Override
-        public Fleetd.AmqpOpener replyInboxOpener() {
-            return (uri, prefetch) -> new ReplyInbox() {
-                @Override public void own(String target) { }
-                @Override public void release(String target) { }
-                @Override public void publish(String target, String msgId, String content) { }
-                @Override public List<InboxMessage> peek(String target) { return List.of(); }
-                @Override public boolean ack(String target, String msgId) { return false; }
-            };
-        }
-
-        @Override
-        public Fleetd.LeadMailboxOpener leadMailboxOpener() {
-            return (uri, selfCoordId, prefetch) -> new FakeLeadChannel();
-        }
-
-        @Override
-        public LongSupplier nanoClock() {
-            return System::nanoTime;
-        }
-
-        @Override
-        public LongSupplier wallClockNanos() {
-            return System::nanoTime;
-        }
-
-        @Override
-        public ScheduledExecutorService newScheduler(String purpose) {
-            return Executors.newSingleThreadScheduledExecutor();
-        }
-
-        @Override
-        public void addShutdownHook(Runnable hook) {
-            shutdownHook = hook;
-        }
-
-        @Override
-        public void startHttp(Javalin app, String host, int port) {
-        }
-
-        @Override
-        public Runnable herdrPollWait() {
-            return () -> {
-                throw new UnsupportedOperationException("FakeHerdr is healthy; no poll wait is expected");
-            };
-        }
-    }
-
-    private TestResourcePorts ports;
-
-    @AfterEach
-    void tearDown() {
-        if (ports != null && ports.shutdownHook != null) {
-            ports.shutdownHook.run();
-        }
-    }
-
-    private static FleetConfig writeConfig(Path dir) throws Exception {
-        Path file = dir.resolve("fleetd.yaml");
-        Files.writeString(file, """
-                bind:
-                  host: 127.0.0.1
-                  port: 8765
-                idleSleepGuard:
-                  enabled: false
-                coordinator:
-                  uri: "amqp://fake-lead-broker/vh"
-                  selfId: "test-lead"
-                """);
-        return FleetConfig.load(file);
-    }
-
-    private FleetdRuntime assemble(Path dir) throws Exception {
-        FleetConfig cfg = writeConfig(dir);
-        ports = new TestResourcePorts();
-        return FleetdAssembly.assembleAndStart(new AssemblyInputs(cfg,
-                new ConfigRef(dir.resolve("fleetd.yaml"), cfg), new SubscriptionGuard(cfg.guard().hostSet())), ports);
-    }
-
-    private static long longField(Object target, String name) throws Exception {
-        Field field = target.getClass().getDeclaredField(name);
-        field.setAccessible(true);
-        return field.getLong(target);
-    }
-
-    @Test
-    void productionBootPathUsesTheExpectedLoopTimingDefaults(@TempDir Path dir) throws Exception {
-        FleetdRuntime runtime = assemble(dir);
-
-        ReplyPushLoop pushLoop = runtime.pushLoop();
-        assertEquals(5, longField(pushLoop, "maxReminders"),
-                "without primary:, ReplyPushLoop must stop after five reminder attempts");
-        assertEquals(15_000L, longField(pushLoop, "backoffMs"),
-                "without primary:, ReplyPushLoop must wait fifteen seconds before the next reminder");
-
-        LeadCoordLoop leadCoordLoop = runtime.leadCoordLoop();
-        assertNotNull(leadCoordLoop, "control: coordinator: must build LeadCoordLoop");
-        assertEquals(3_000L, longField(leadCoordLoop, "intervalMs"),
-                "LeadCoordLoop must poll for peer-lead mail every three seconds");
-
-        StatusPoller poller = runtime.poller();
-        assertEquals(Injector.POLL_INTERVAL_MILLIS, longField(poller, "intervalMillis"),
-                "StatusPoller must use Injector's delivery poll interval");
-    }
-}

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#://[^@]*@#://<redacted>@#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

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://<redacted>@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 =="