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. + * + *
The blanking loop also re-asserts, on its own, the same {@code [A-Za-z_][A-Za-z0-9_]*} shape
+ * check the enumeration loop already applied. Before {@code eval} was introduced a non-conforming
+ * name reaching {@code export "$n="} was harmless either way — the quoting made it inert. With
+ * {@code eval}, the name is spliced into a string and interpreted as shell syntax, so the enumeration
+ * loop's check is no longer sufficient on its own to keep that call site safe — it is a guard on a
+ * different loop, and the two must not silently drift apart. Re-checking right before the
+ * {@code eval} keeps that call site safe by its own reading, independent of whatever the enumeration
+ * loop does or stops doing in a later change.
*/
public final class EnvAllowListScrub {
@@ -132,10 +151,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");
+ }
+
+ /**
+ * fleetd #394 follow-up: the blanking loop's {@code eval "export ${n}="} splices {@code n} into
+ * a string that zsh then interprets as shell syntax. That is only safe because every name
+ * reaching {@code _cb633_blank} already passed an identifier check in the ENUMERATION loop
+ * (20 lines away, in a different loop) — so the fix re-asserts the identical check immediately
+ * before the {@code eval} call, rather than trusting that distant guard to keep holding.
+ *
+ * This test plants a value with an embedded newline, exploiting the exact "junk from
+ * multi-line values" gap the enumeration loop's own comment already documents: {@code command
+ * env}'s text output is read line-by-line, so a value's second line becomes a spurious extra
+ * "name" that was never a real exported variable. The fragment used here ({@code
+ * junk.fragment}) is merely non-conforming (it contains a dot) — never command-shaped; this
+ * test must never demonstrate command execution and plants no command-shaped payload.
+ *
+ * Exercises the real artefact end-to-end: {@link EnvAllowListScrub#scrubScript} runs
+ * verbatim under a real {@code /bin/zsh}, exactly as {@code generate()} would produce it — this
+ * is not a synthetic call into just the blanking loop.
+ */
+ @Test
+ void nonIdentifierJunkFromAMultilineValueIsSkippedNotBlankedOrUnblankable(@TempDir Path tmp)
+ throws Exception {
+ assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
+
+ String script = EnvAllowListScrub.scrubScript(Set.of("ZDOTDIR"));
+
+ ProcessBuilder pb = new ProcessBuilder("/bin/zsh");
+ pb.environment().clear();
+ pb.environment().put("PATH", "/usr/bin:/bin");
+ pb.environment().put("ZDOTDIR", tmp.toAbsolutePath().toString());
+ // Embedded newline: `command env`'s own text output splits this into two lines, and the
+ // second ("junk.fragment") has no "=" at all, so `cut -d= -f1` returns it unchanged as a
+ // spurious candidate "name" — it was never an actual exported variable by that name.
+ pb.environment().put("FLEETD_TEST_MULTILINE", "keep\njunk.fragment");
+ pb.redirectError(ProcessBuilder.Redirect.DISCARD);
+ Process zsh = pb.start();
+ zsh.getOutputStream().write(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 must reach its end");
+
+ EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(tmp);
+ assertNotNull(report, "the report must still be written");
+ assertTrue(report.blanked().contains("FLEETD_TEST_MULTILINE"),
+ "sanity: the real, identifier-shaped variable must still be blanked normally");
+ assertFalse(report.blanked().contains("junk.fragment"),
+ "a non-identifier fragment is not a real variable and must never be blanked");
+ assertFalse(report.unblankable().contains("junk.fragment"),
+ "a non-identifier fragment must never even become a candidate the blanking loop "
+ + "attempts — it must be filtered before either guard has to catch it, so "
+ + "it is neither blanked nor counted as a failed attempt");
+ }
+
/** A group-shared ZDOTDIR still lets the member truncate and write its pre-created receipt. */
@Test
void groupSharedScrubWritesAndReadsItsReport(@TempDir Path tmp) throws Exception {