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 ea3793d..d56105b 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -13,6 +13,7 @@ import java.nio.file.attribute.PosixFilePermissions; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.stream.Stream; @@ -28,24 +29,46 @@ import java.util.stream.Stream; * control lacked: herdr applies that overlay BEFORE the shell starts, so any sourced file can undo * it — and did. * - *

Which file is last depends on the platform, so the scrub runs from two of them. zsh + *

zsh reads its four startup files under three different conditions, so no single file is + * guaranteed to run — the scrub has to cover the gap between them, not just the platforms. zsh * reads {@code .zshenv} always, {@code .zprofile} and {@code .zlogin} only for a LOGIN shell, and - * {@code .zshrc} only for an INTERACTIVE one. herdr does not open the same kind of shell - * everywhere — measured on herdr 0.8.0: macOS panes run {@code -zsh} (login, so {@code .zlogin} - * runs), Linux panes run a plain {@code /usr/bin/zsh} (interactive but NOT login, so - * {@code .zlogin} never runs at all). A scrub in {@code .zlogin} alone is therefore a control that - * silently does nothing on Linux — the exact failure this class exists to remove, one platform - * over. + * {@code .zshrc} only for an INTERACTIVE one. A pane shell that is at least one of login or + * interactive is covered by sourcing the scrub from {@code .zshrc} and {@code .zlogin} (below), but + * a pane shell that is NEITHER reads only {@code .zshenv} and stops — fleetd #388, measured: a herdr + * pane can be neither login nor interactive, and such a pane read {@code .zshenv}, never reached + * {@code scrub.zsh}, and left no report at all. A bare {@code argv[0]} of {@code /usr/bin/zsh} + * proves the shell is NOT a login shell; it says nothing about whether it is interactive, so it + * must never be read as "therefore interactive" — that wrong inference is what let #388 ship. * - *

So both {@code .zshrc} and {@code .zlogin} source the same generated {@code scrub.zsh} after - * sourcing their {@code $HOME} counterpart. On Linux only the first fires; on macOS both do, and - * the second pass is deliberate rather than merely harmless — it re-scrubs anything the operator's - * own {@code ~/.zlogin} exported after {@code .zshrc} had finished. Re-running is idempotent: a - * name already blank is blanked again, and the report is rewritten with the same counts. + *

So {@code .zshenv} carries a THIRD pass, guarded by the exact condition that defines the gap: + * {@code [[ ! -o login && ! -o interactive ]]}. That guard is why this pass cannot double-scrub a + * pane that {@code .zshrc} or {@code .zlogin} will also cover — one of {@code -o login}/ + * {@code -o interactive} is always true there, so the {@code .zshenv} pass never fires for them, and + * their own unconditional sourcing is untouched. The guard also carries a sentinel + * ({@value #SCRUB_SENTINEL}) so it fires once per PANE and not once per PROCESS: {@code .zshenv} is + * read by every zsh a member's own tooling forks (a plain {@code zsh -c '...'} for a single + * command is itself neither login nor interactive), and those children inherit variables their + * parent deliberately set for them (git hooks get {@code GIT_DIR}, a venv gets + * {@code VIRTUAL_ENV}, a build tool gets {@code NODE_OPTIONS} or {@code JAVA_TOOL_OPTIONS}). + * Re-scrubbing every such child would blank all of that, and would also make the pane's own + * {@code scrub-report.txt} — rewritten on every pass — describe whichever child exited last + * instead of the pane. The sentinel is exported only AFTER {@code scrub.zsh} runs, so the pass + * that sets it never sees it and cannot blank it; it must also be on the scrub's own allow-list + * (see {@link #generate(Path, Set)}) so a later pass, in the same pane, cannot blank it back to + * empty — an exported-but-empty sentinel reads as unset to the {@code -z} guard and would silently + * re-enable scrubbing for every subsequent child of that pane. + * + *

So all three of {@code .zshenv} (gap only, guarded), {@code .zshrc}, and {@code .zlogin} + * source the same generated {@code scrub.zsh} after sourcing their {@code $HOME} counterpart. A + * login-and-interactive pane runs the {@code .zshrc} and {@code .zlogin} passes, and the second is + * deliberate rather than merely harmless — it re-scrubs anything the operator's own + * {@code ~/.zlogin} exported after {@code .zshrc} had finished. A pane that is neither runs only the + * {@code .zshenv} pass. Re-running is idempotent: a name already blank is blanked again, and the + * report is rewritten with the same counts. * *

Each generated file sources its {@code $HOME} counterpart FIRST, so {@code PATH} and every - * toolchain binary still resolve exactly as the operator configured them; only afterwards does - * {@code .zlogin} run the scrub: every EXPORTED variable not on the derived allow-list is re-exported + * toolchain binary still resolve exactly as the operator configured them; only afterwards does the + * scrub run: every EXPORTED variable not on the derived allow-list is re-exported * blank. Blank, not credential-shaped-pattern-filtered: a pattern list ({@code *TOKEN*}, …) is an * enumeration and misses what it did not think of — a username is the other half of a credential and * is shaped like none. Credential-SHAPED names among the blanked set go to the WARN log only, @@ -63,7 +86,11 @@ public final class EnvAllowListScrub { /** Name of the report file written into the generated directory by the scrub itself. */ static final String REPORT_FILE = "scrub-report.txt"; - /** The scrub body, generated once and sourced from both {@code .zshrc} and {@code .zlogin}. */ + /** + * The scrub body, generated once and sourced from {@code .zshrc} and {@code .zlogin} + * unconditionally, and from {@code .zshenv} when the pane shell is neither login nor + * interactive (fleetd #388) — see the class javadoc. + */ static final String SCRUB_FILE = "scrub.zsh"; /** Prefix of every generated directory — also what {@link #reapOrphans} matches on. */ @@ -79,6 +106,28 @@ public final class EnvAllowListScrub { private static final String SOURCE_SCRUB = "source \"$ZDOTDIR/" + SCRUB_FILE + "\"\n"; + /** + * fleetd #388: marks a pane, not a process, as already scrubbed. Set only by the guarded + * {@code .zshenv} pass (see {@link #NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB}) after + * {@code scrub.zsh} has run, so it must also be folded into that pass's own allow-list — see + * the class javadoc's "must also be on the scrub's own allow-list" paragraph. + */ + static final String SCRUB_SENTINEL = "_CB633_SCRUBBED"; + + /** + * Appended to {@code .zshenv}, after its {@code $HOME} source: the third pass, guarded on the + * exact condition that defines the gap {@code .zshrc}/{@code .zlogin} do not cover — a shell + * that is neither login nor interactive. The sentinel export happens only once the scrub has + * already run, and only for as long as the current pane's environment has not been rebuilt from + * scratch (a fresh {@code env -i} child would not inherit it — that is out of scope here, since + * such a child is no longer running under the pane's own environment at all). + */ + private static final String NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB = + "if [[ ! -o login && ! -o interactive && -z \"${" + SCRUB_SENTINEL + ":-}\" ]]; then\n" + + " " + SOURCE_SCRUB + + " export " + SCRUB_SENTINEL + "=1\n" + + "fi\n"; + private EnvAllowListScrub() { } @@ -108,8 +157,13 @@ public final class EnvAllowListScrub { // zsh truncates this pre-created receipt after these hooks are registered. Register its // path too — otherwise the directory is non-empty at JVM exit and cannot be removed. Files.createFile(dir.resolve(REPORT_FILE)).toFile().deleteOnExit(); - write(dir, SCRUB_FILE, scrubScript(allowedNames)); - write(dir, ".zshenv", homeSourcingFile(".zshenv")); + // fleetd #388: scrub.zsh's OWN allow-list must also keep SCRUB_SENTINEL, or a later + // pass in the same pane blanks it back to empty and the .zshenv guard below thinks it + // was never scrubbed — see the class javadoc. + Set namesForScrubScript = new HashSet<>(allowedNames); + namesForScrubScript.add(SCRUB_SENTINEL); + write(dir, SCRUB_FILE, scrubScript(namesForScrubScript)); + write(dir, ".zshenv", homeSourcingFile(".zshenv") + NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB); write(dir, ".zprofile", homeSourcingFile(".zprofile")); write(dir, ".zshrc", homeSourcingFile(".zshrc") + SOURCE_SCRUB); write(dir, ".zlogin", homeSourcingFile(".zlogin") + SOURCE_SCRUB); @@ -252,11 +306,13 @@ public final class EnvAllowListScrub { } return """ # generated by fleetd (CB-633 memberCredentials policy=allow-list) — do not edit. - # Sourced from .zshrc and again from .zlogin, each time AFTER that file has sourced - # its $HOME counterpart — so this runs after everything the operator sourced, on a - # login shell (macOS panes) and on a plain interactive one (Linux panes) alike. - # Running twice is idempotent and deliberate: the second pass catches anything - # ~/.zlogin exported after ~/.zshrc had finished. + # Sourced unconditionally from .zshrc and again from .zlogin, each time AFTER that + # file has sourced its $HOME counterpart — so this runs after everything the + # operator sourced, on any pane that is login and/or interactive. Running twice is + # idempotent and deliberate: the second pass catches anything ~/.zlogin exported + # after ~/.zshrc had finished. Also sourced, once, from a guarded pass in .zshenv + # (fleetd #388) when the pane shell is NEITHER login nor interactive — the one gap + # those two files do not cover. typeset -A _cb633_allowed for _cb633_n in %s; do _cb633_allowed[$_cb633_n]=1; done diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java index be4125e..6d16bba 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java @@ -8,6 +8,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -45,6 +46,15 @@ class EnvAllowListScrubTest { /** Env var names appearing in command output; anything else (prompts, wrapped lines) is noise. */ private static final Pattern ENV_NAME = Pattern.compile("^([A-Za-z_][A-Za-z0-9_]*)$"); + /** + * fleetd #388: the sentinel name the generated {@code .zshenv} guard uses, kept here as a + * literal rather than referencing {@link EnvAllowListScrub#SCRUB_SENTINEL} — the two tests that + * use it must still compile and run against the pre-fix production class (which has no such + * constant), so the revert-and-prove-it-fails step exercises a real assertion instead of a + * compilation error. + */ + private static final String SCRUB_SENTINEL_NAME = "_CB633_SCRUBBED"; + /** * The equality test. Expected survivors = baseline exports ∩ allowed — i.e. every survivor is * allowed AND every allowed name that existed survives. The operator's own secret-store exports @@ -249,6 +259,140 @@ class EnvAllowListScrubTest { + "shell. A difference here means the scrub is dead on Linux."); } + /** + * fleetd #388: the actual gap. zsh reads {@code .zshenv} always, {@code .zprofile}/ + * {@code .zlogin} only for a LOGIN shell, and {@code .zshrc} only for an INTERACTIVE one — so a + * shell that is NEITHER (a bare {@code /bin/zsh} reading a script off a non-tty stdin, no + * {@code -l}, no {@code -i}) reads only {@code .zshenv} and stops. Before this fix, that shell + * never reached {@code scrub.zsh} at all: the decoy secret below would survive untouched. This + * test injects that decoy directly into the process environment (not via a sourced dotfile, + * since the whole point of the gap is that {@code .zshenv} is normally close to empty) so the + * test does not depend on any real {@code ~/.zshrc} content existing on the host. + */ + @Test + void scrubRunsInAShellThatIsNeitherLoginNorInteractive(@TempDir Path tmp) throws Exception { + assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here"); + Set allowed = MemberEnvAllowList.derive(List.of()); + Path zdotdir = EnvAllowListScrub.generate(tmp, allowed); + + Map cleanParent = new HashMap<>(Map.of( + "HOME", System.getProperty("user.home"), + "PATH", "/usr/bin:/bin", + "SHELL", "/bin/zsh", + "USER", System.getProperty("user.name", "nobody"), + "TMPDIR", tmp.toString())); + cleanParent.put("FLEETD_TEST_DECOY_SECRET", "x"); // not on any allow-list; must be blanked + + List neither = List.of(); // no -l, no -i; stdin is a pipe (never a tty) either way + Set baseline = exportedNamesFromCleanParent(cleanParent, null, neither); + Set scrubbed = exportedNamesFromCleanParent(cleanParent, zdotdir, neither); + + Set expected = new TreeSet<>(); + for (String name : baseline) { + if (MemberEnvAllowList.keeps(allowed, name)) { + expected.add(name); + } + } + assertTrue(baseline.contains("FLEETD_TEST_DECOY_SECRET"), + "sanity: the decoy must actually reach the un-scrubbed baseline, or this test proves " + + "nothing"); + expected.add("ZDOTDIR"); // the harness set it and it is infrastructure, so it must survive + expected.add(SCRUB_SENTINEL_NAME); // set by the new .zshenv guard once scrubbed + assertEquals(expected, scrubbed, + "a pane shell that is NEITHER login nor interactive must still be scrubbed — its " + + "surviving exported names must EQUAL baseline ∩ allow-list, plus the " + + "sentinel the guard sets once it has run. FLEETD_TEST_DECOY_SECRET surviving " + + "here means the gap is still open."); + } + + /** + * fleetd #388 invariants 3 and 4, which a name-set equality cannot show: a member's own tooling + * forks plain, non-login, non-interactive zsh processes for a single command (the same shape as + * the pane shell itself), and such a child must (a) keep whatever its parent deliberately set + * for it, never (b) re-run the scrub and blank it, and never (c) overwrite the pane's own + * {@code scrub-report.txt} with a description of itself instead of the pane. All three can only + * be shown by actually running a child process from within the scrubbed pane shell. + * + *

The pane and the child both report presence via {@code ${NAME:+present}} — empty when a + * name is unset OR blanked (exported empty), {@code present} when it is set and non-empty. No + * value is ever printed, only these two shapes and the literal word {@code set}/{@code unset} + * for the sentinel. + */ + @Test + void neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane(@TempDir Path tmp) throws Exception { + assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here"); + Set allowed = MemberEnvAllowList.derive(List.of()); + Path zdotdir = EnvAllowListScrub.generate(tmp, allowed); + + Map paneEnv = new HashMap<>(Map.of( + "HOME", System.getProperty("user.home"), + "PATH", "/usr/bin:/bin", + "SHELL", "/bin/zsh", + "USER", System.getProperty("user.name", "nobody"), + "TMPDIR", tmp.toString())); + paneEnv.put("FLEETD_TEST_DECOY_SECRET", "x"); // not allow-listed; the pane must blank it + paneEnv.put("ZDOTDIR", zdotdir.toAbsolutePath().toString()); + + // The pane's own script reports what IT sees, then forks a plain non-login, non-interactive + // child — the shape a member's own tooling uses — carrying a variable the "parent" (this + // pane) deliberately set for it, the way git sets GIT_DIR for a hook. + String outerScript = """ + print -r -- "PANE_SENTINEL=${%1$s:+set}" + print -r -- "PANE_DECOY=${FLEETD_TEST_DECOY_SECRET:+present}" + FLEETD_TEST_TOOL_VAR=keep /bin/zsh <<'CHILD' + print -r -- "CHILD_LOGIN=$([[ -o login ]] && echo yes || echo no)" + print -r -- "CHILD_INTERACTIVE=$([[ -o interactive ]] && echo yes || echo no)" + print -r -- "CHILD_TOOL_VAR=${FLEETD_TEST_TOOL_VAR:+present}" + print -r -- "CHILD_DECOY=${FLEETD_TEST_DECOY_SECRET:+present}" + print -r -- "CHILD_SENTINEL=${%1$s:+set}" + CHILD + exit + """.formatted(SCRUB_SENTINEL_NAME); + + ProcessBuilder pb = new ProcessBuilder("/bin/zsh"); // no -l, no -i: the pane's own shape + pb.environment().clear(); + pb.environment().putAll(paneEnv); + pb.redirectError(ProcessBuilder.Redirect.DISCARD); + Process zsh = pb.start(); + zsh.getOutputStream().write(outerScript.getBytes(StandardCharsets.UTF_8)); + zsh.getOutputStream().flush(); + zsh.getOutputStream().close(); + String stdout = new String(zsh.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertTrue(zsh.waitFor(60, java.util.concurrent.TimeUnit.SECONDS), + "the pane+child probe did not exit within 60s"); + assertTrue(zsh.exitValue() == 0, "probe zsh exited non-zero: " + stdout); + + Map reported = new HashMap<>(); + for (String line : stdout.split("\n")) { + int eq = line.indexOf('='); + if (eq > 0) { + reported.put(line.substring(0, eq).trim(), line.substring(eq + 1).trim()); + } + } + + assertEquals("set", reported.get("PANE_SENTINEL"), + "the pane itself is neither login nor interactive, so the .zshenv guard must have " + + "run the scrub and exported the sentinel"); + assertEquals("", reported.get("PANE_DECOY"), + "the pane must blank a non-allow-listed name — invariant 1"); + assertEquals("no", reported.get("CHILD_LOGIN"), "sanity: the child must also be non-login"); + assertEquals("no", reported.get("CHILD_INTERACTIVE"), "sanity: the child must also be non-interactive"); + assertEquals("present", reported.get("CHILD_TOOL_VAR"), + "invariant 3: a variable the pane deliberately set for its child must survive — a " + + "child that re-ran the scrub would have blanked it"); + assertEquals("", reported.get("CHILD_DECOY"), + "a name already blanked by the pane must stay blanked in the child, never resurrected"); + assertEquals("set", reported.get("CHILD_SENTINEL"), + "the child must inherit the sentinel from the pane's environment, or it would re-scrub"); + + EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir); + assertNotNull(report, "the pane's own scrub pass must leave a report behind"); + assertTrue(report.blanked().stream().noneMatch(n -> n.startsWith("FLEETD_TEST_TOOL_VAR")), + "invariant 4: the receipt must still describe the PANE, not the child — a child that " + + "re-ran the scrub would have rewritten this file to list its own " + + "FLEETD_TEST_TOOL_VAR as blanked"); + } + /** * Run {@code /bin/zsh -l -i} from a clean parent and return the NAMES it has exported by prompt * time. With {@code zdotdir} non-null, {@code ZDOTDIR} points at a generated scrub directory, so