#394: contain a fatal export error instead of letting it abort the scrub

EnvAllowListScrub's blanking loop used a plain `export "$n="` on every
name not on the allow-list. For a zsh read-only/special parameter (e.g.
UID) that is a FATAL parameter error, and since the loop runs inside the
sourced startup file, the error aborts the whole file: every name still
to come is never blanked, and scrub-report.txt is never written at all
-- silently, because 2>/dev/null on the group swallows it.

Route each blanking attempt through `eval` instead, which contains the
error to that one iteration. The loop always finishes; a name it could
not blank is now counted separately ("failed" on the report's first
line) and listed !-prefixed rather than disappearing. No skip-list of
known-bad names is added -- every enumerated name is still attempted,
so a name nobody has thought of is still tried and, if it fails, still
counted.

HerdrPeerLauncher: log a WARN when a pane's report carries a nonzero
failed count, and reword the "no report at all" WARN so it no longer
claims the daemon knows the member "saw the full host environment" --
a partial vs. a missing scrub are different situations and only the
first is now distinguishable from the report alone.

fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java

@@ -75,9 +75,19 @@ import java.util.stream.Stream;
  * never to the control.
  *
  * <p>The scrub also writes {@code scrub-report.txt} into its own directory: one {@code allowed N of
- * M} line (N = exports left untouched, M = exports present when the scrub ran), then the blanked
- * NAMES — never values. The launcher reads this back at teardown and logs it, because a blocked
- * count next to an unknown denominator is not a finding.
+ * M failed F} line (N = exports left untouched, M = exports present when the scrub ran, F = names
+ * the scrub attempted to blank but could not), then the NAMES — blanked ones bare, unblankable ones
+ * {@code !}-prefixed — never values. The launcher reads this back at teardown and logs it, because a
+ * blocked count next to an unknown denominator is not a finding.
+ *
+ * <p><b>fleetd #394:</b> plain {@code export "$n="} is a FATAL error for a zsh read-only or special
+ * parameter (for example {@code UID}) — it aborts the whole sourced file, so every name still to
+ * come is never blanked and the report above is never written at all. The blanking loop instead
+ * routes each attempt through {@code eval}, which contains that error to the single iteration: the
+ * loop always finishes, and a name that could not be blanked is counted as {@code failed} and
+ * listed {@code !}-prefixed rather than silently disappearing. This is deliberately not a skip-list
+ * of known-bad names — every enumerated name is still attempted, so a name nobody has thought of
+ * yet still gets tried and, if it fails, still gets counted.
  */
 public final class EnvAllowListScrub {
 
@@ -132,10 +142,16 @@ public final class EnvAllowListScrub {
     }
 
     /**
-     * A parsed {@code scrub-report.txt}: how many exported variables existed when the scrub ran,
-     * how many were left untouched (allowed), and the NAMES that were blanked. Values never appear.
+     * A parsed {@code scrub-report.txt}: how many exported variables existed when the scrub ran
+     * ({@code total}), how many were left untouched ({@code allowed}), how many the scrub attempted
+     * to blank but could not ({@code failed} — fleetd #394: a zsh read-only/special parameter such
+     * as {@code UID} fatally errors on plain {@code export NAME=}, so those attempts go through
+     * {@code eval} instead so the loop keeps going and the failure is counted rather than left
+     * invisible), and the NAMES in each of the latter two categories. {@code allowed +
+     * blanked.size() + unblankable.size() == total}, and {@code unblankable.size() == failed}.
+     * Values never appear.
      */
-    record ScrubReport(int allowed, int total, List<String> blanked) {
+    record ScrubReport(int allowed, int total, int failed, List<String> blanked, List<String> unblankable) {
     }
 
     /**
@@ -334,15 +350,32 @@ public final class EnvAllowListScrub {
                   _cb633_blank+=("$_cb633_n")
                 done
 
-                { for _cb633_n in "${_cb633_blank[@]}"; do export "$_cb633_n="; done; } 2>/dev/null
+                # fleetd #394: plain `export "$n="` is FATAL for a zsh read-only/special parameter
+                # (e.g. UID) and aborts this whole sourced file — every name still to come is never
+                # blanked, and the report below is never written, silently. `eval` contains that
+                # error to the single iteration instead: it still fails for that one name, but the
+                # loop continues and we can tell allowed / blanked / unblankable apart afterwards.
+                # This is not a skip-list of known-bad names (that would miss the next one nobody
+                # thought of) — every name in _cb633_blank is still attempted, unconditionally.
+                typeset -a _cb633_ok _cb633_unblankable
+                _cb633_ok=()
+                _cb633_unblankable=()
+                for _cb633_n in "${_cb633_blank[@]}"; do
+                  if eval "export ${_cb633_n}=" 2>/dev/null; then
+                    _cb633_ok+=("$_cb633_n")
+                  else
+                    _cb633_unblankable+=("$_cb633_n")
+                  fi
+                done
 
                 integer _cb633_kept=$(( _cb633_total - ${#_cb633_blank} ))
                 {
-                  print -r -- "allowed $_cb633_kept of $_cb633_total"
-                  for _cb633_n in "${_cb633_blank[@]}"; do print -r -- "$_cb633_n"; done
+                  print -r -- "allowed $_cb633_kept of $_cb633_total failed ${#_cb633_unblankable}"
+                  for _cb633_n in "${_cb633_ok[@]}"; do print -r -- "$_cb633_n"; done
+                  for _cb633_n in "${_cb633_unblankable[@]}"; do print -r -- "!$_cb633_n"; done
                 } > "$ZDOTDIR/%s" 2>/dev/null
 
-                unset _cb633_allowed _cb633_names _cb633_blank _cb633_n _cb633_total _cb633_kept
+                unset _cb633_allowed _cb633_names _cb633_blank _cb633_ok _cb633_unblankable _cb633_n _cb633_total _cb633_kept
                 """.formatted(names, MemberEnvAllowList.zshCasePattern(), REPORT_FILE);
     }
 
@@ -356,6 +389,11 @@ public final class EnvAllowListScrub {
      * Read and parse {@link #REPORT_FILE} out of a generated ZDOTDIR directory. Returns {@code null}
      * when absent or unreadable (the pane may have been torn down before its login shell ever got to
      * the scrub) — callers treat that as "no measurement available", never as success.
+     *
+     * <p>First line is {@code "allowed <N> of <M> failed <F>"} (fleetd #394 added the trailing
+     * {@code failed <F>} — a count of names the scrub attempted to blank but could not, e.g. a zsh
+     * read-only/special parameter). Every following non-blank line is a name: a bare name was
+     * blanked, a {@code !}-prefixed name was attempted and failed. Values never appear on either.
      */
     static ScrubReport readReport(Path zdotdir) {
         Path report = zdotdir.resolve(REPORT_FILE);
@@ -368,17 +406,24 @@ public final class EnvAllowListScrub {
                 return null;
             }
             String[] parts = lines.getFirst().substring("allowed ".length()).trim().split("\\s+");
-            if (parts.length != 3 || !"of".equals(parts[1])) {
+            if (parts.length != 5 || !"of".equals(parts[1]) || !"failed".equals(parts[3])) {
                 return null;
             }
             List<String> blanked = new ArrayList<>();
+            List<String> unblankable = new ArrayList<>();
             for (int i = 1; i < lines.size(); i++) {
-                if (!lines.get(i).isBlank()) {
-                    blanked.add(lines.get(i));
+                String line = lines.get(i);
+                if (line.isBlank()) {
+                    continue;
+                }
+                if (line.startsWith("!")) {
+                    unblankable.add(line.substring(1));
+                } else {
+                    blanked.add(line);
                 }
             }
             return new ScrubReport(Integer.parseInt(parts[0]), Integer.parseInt(parts[2]),
-                    List.copyOf(blanked));
+                    Integer.parseInt(parts[4]), List.copyOf(blanked), List.copyOf(unblankable));
         } catch (IOException | NumberFormatException e) {
             return null;
         }

fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java

@@ -1643,11 +1643,19 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
             log.warn("memberCredentials allow-list: pane {} left no scrub report in {} — the "
                     + "environment scrub cannot be confirmed to have run. Either the pane ended "
                     + "before its shell finished starting, or its shell never read our generated "
-                    + "startup files, in which case that member saw the full host environment.",
+                    + "startup files. Either way, we cannot tell from here whether the scrub ran, "
+                    + "so we do not know what that member's environment contained.",
                     paneId, dir);
         } else {
             log.info("memberCredentials allow-list: pane {} allowed {} of {} environment variables",
                     paneId, report.allowed(), report.total());
+            if (report.failed() > 0) {
+                log.warn("memberCredentials allow-list: pane {} could not blank {} environment "
+                        + "variable(s) — {} (likely a zsh read-only/special parameter) — those "
+                        + "names were left in the member's environment. Confirm none of them is a "
+                        + "credential.",
+                        paneId, report.failed(), report.unblankable());
+            }
             List<String> shaped = report.blanked().stream()
                     .filter(name -> CREDENTIAL_SHAPED_NAME.matcher(name).matches())
                     .toList();

fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java

@@ -118,6 +118,77 @@ class EnvAllowListScrubTest {
                 "allowed N of M with N <= M — the denominator is always reported");
     }
 
+    /**
+     * fleetd #394: the actual defect. Plain {@code export "$n="} is FATAL for a zsh read-only or
+     * special parameter (e.g. {@code UID}) and aborts the whole sourced file — every name still to
+     * come is never blanked, and the {@code scrub-report.txt} below is never written at all,
+     * silently ({@code 2>/dev/null} swallows the error). This plants an unblankable, exported,
+     * read-only variable in the MIDDLE of the names the scrub attempts to blank, with two more
+     * names after it, and asserts that both of those later names are STILL blanked and the report
+     * is STILL written with the failure counted — a test that only checked names BEFORE the failure
+     * point would pass today and prove nothing.
+     *
+     * <p>The planted name is a made-up one ({@code FLEETD_TEST_UNBLANKABLE}), not {@code UID} or
+     * any other name a skip-list might already know about — invariant 1 is that the loop survives
+     * ANY unblankable name, not a known one, so the test must not lean on one either.
+     *
+     * <p>Exercises the real artefact: {@link EnvAllowListScrub#scrubScript} is run verbatim under a
+     * real {@code /bin/zsh}, not just asserted on as a Java string. The four planted names are
+     * exported one at a time via {@code typeset -x}/{@code typeset -rx} immediately before the
+     * script runs, in a fixed order — zsh's {@code export}/{@code typeset -x} appends to the
+     * process's environment table in call order (verified empirically: a freshly-exported name
+     * always sorts after every inherited one and after every earlier freshly-exported name in
+     * {@code command env}'s own output), which is what makes the "middle" position deterministic
+     * here, unlike relying on the OS's own inherited-environment order.
+     */
+    @Test
+    void unblankableNameInTheMiddleDoesNotAbortNamesAfterIt(@TempDir Path tmp) throws Exception {
+        assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
+
+        // Only ZDOTDIR is allowed — it must survive the scrub itself, since the report is written
+        // to "$ZDOTDIR/..." AFTER the blanking loop runs; if ZDOTDIR were blanked as a side effect,
+        // the report write would silently go to the wrong place instead of testing anything.
+        String script = EnvAllowListScrub.scrubScript(Set.of("ZDOTDIR"));
+        String setup = """
+                typeset -x FLEETD_TEST_BEFORE=1
+                typeset -rx FLEETD_TEST_UNBLANKABLE=1
+                typeset -x FLEETD_TEST_AFTER_A=1
+                typeset -x FLEETD_TEST_AFTER_B=1
+                """;
+
+        ProcessBuilder pb = new ProcessBuilder("/bin/zsh");
+        pb.environment().clear();
+        pb.environment().put("PATH", "/usr/bin:/bin");
+        pb.environment().put("ZDOTDIR", tmp.toAbsolutePath().toString());
+        pb.redirectError(ProcessBuilder.Redirect.DISCARD);
+        Process zsh = pb.start();
+        zsh.getOutputStream().write((setup + script).getBytes(StandardCharsets.UTF_8));
+        zsh.getOutputStream().flush();
+        zsh.getOutputStream().close();
+        assertTrue(zsh.waitFor(60, java.util.concurrent.TimeUnit.SECONDS),
+                "the scrub script did not exit within 60s");
+        assertEquals(0, zsh.exitValue(),
+                "the scrub script itself must never abort — an unblankable name must not kill the "
+                        + "sourced file");
+
+        EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(tmp);
+        assertNotNull(report, "the report must still be written even though one name could not be "
+                + "blanked — a report that silently never appears is the #394 bug");
+        assertTrue(report.blanked().contains("FLEETD_TEST_BEFORE"),
+                "sanity: the name before the unblankable one must be blanked");
+        assertTrue(report.blanked().contains("FLEETD_TEST_AFTER_A"),
+                "the FIRST name AFTER the unblankable one must still be blanked — before the fix, "
+                        + "the whole loop aborted at the unblankable name and every later name was "
+                        + "silently left untouched");
+        assertTrue(report.blanked().contains("FLEETD_TEST_AFTER_B"),
+                "the SECOND name after the unblankable one must also still be blanked");
+        assertEquals(1, report.failed(),
+                "exactly one attempted name could not be blanked, and that count must be visible "
+                        + "without reading the member's environment");
+        assertEquals(List.of("FLEETD_TEST_UNBLANKABLE"), report.unblankable(),
+                "the unblankable name is reported by name, not silently dropped");
+    }
+
     /** A group-shared ZDOTDIR still lets the member truncate and write its pre-created receipt. */
     @Test
     void groupSharedScrubWritesAndReadsItsReport(@TempDir Path tmp) throws Exception {