diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java index d56105b..b2ba33e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -75,9 +75,19 @@ import java.util.stream.Stream; * never to the control. * *
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. + * + *
fleetd #394: 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 First line is {@code "allowed 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.
+ *
+ * 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 {