CB-633: allow-list. Deny-by-default's overlay is applied BEFORE the pane's login * shell runs, so any file that chain sources can re-export over it — and did. The allow-list - * policy moves the control to a generated ZDOTDIR whose {@code .zlogin} runs LAST, after the - * whole operator chain, and blanks every exported variable not on an allow-list DERIVED from + * policy moves the control to a generated ZDOTDIR whose startup files run the scrub LAST, after + * the whole operator chain, and blank every exported variable not on an allow-list DERIVED from * what the launcher itself injects (profiles' tokenEnv/gitTokenEnv/gitHostEnv/env keys plus an * infrastructure set) — never hand-typed, so adding a profile cannot break a spawn. Under this * policy {@link #allow} and {@link #known} stop being a control and become reporting only. @@ -1005,8 +1005,10 @@ public record FleetConfig( /** * CB-633: derive the kept-name set from what the launcher itself injects, generate a - * per-spawn ZDOTDIR whose {@code .zlogin} blanks every exported variable not on it AFTER the - * pane's login shell has finished sourcing the operator's chain. + * per-spawn ZDOTDIR whose startup files blank every exported variable not on it AFTER the + * pane's shell has finished sourcing the operator's chain. The scrub is sourced from both + * the generated {@code .zshrc} and {@code .zlogin}, because herdr opens a LOGIN zsh on macOS + * and a plain interactive one on Linux — see {@code EnvAllowListScrub}. */ public static final String POLICY_ALLOW_LIST = "allow-list"; @@ -1017,7 +1019,7 @@ public record FleetConfig( public MemberCredentials { String normalizedPolicy = (policy == null || policy.isBlank()) - ? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(); + ? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(java.util.Locale.ROOT); // deny-list is an alias of deny-by-default, not a third behaviour — normalize to one // spelling so every isDenyList()-style check has one value to compare against. policy = POLICY_DENY_LIST.equals(normalizedPolicy) ? POLICY_DENY_BY_DEFAULT : normalizedPolicy; diff --git a/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java b/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java index 5cc4812..be8839c 100644 --- a/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java +++ b/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -1,9 +1,14 @@ package dev.ltms.fleet.member; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -13,12 +18,27 @@ import java.util.stream.Stream; * CB-633: generates the per-spawn {@code ZDOTDIR} directory whose startup files enforce * {@code memberCredentials.policy: allow-list}. * - *
The seam: a login interactive zsh sources {@code $ZDOTDIR/.zshenv}, then {@code .zprofile}, - * then {@code .zshrc}, then {@code .zlogin} — in that order, LAST first-named-last. The operator's - * whole chain ({@code ~/.zshrc} → secret store) runs inside the first three, so a scrub placed in - * the generated {@code .zlogin} runs after everything the operator sourced, and nothing later can - * re-export over it. This is the property CB-596's env-overlay control lacked: herdr applies that - * overlay BEFORE the login shell starts, so any sourced file can undo it — and did. + *
The seam: zsh reads its startup files from {@code $ZDOTDIR}, and the daemon puts that variable + * in the pane-creation env map. The operator's whole chain ({@code ~/.zshrc} → secret store) runs + * inside those files, so a scrub appended to the LAST one runs after everything the operator + * sourced, and nothing later can re-export over it. This is the property CB-596's env-overlay + * 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 + * 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. + * + *
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. * *
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
@@ -35,9 +55,27 @@ import java.util.stream.Stream;
*/
public final class EnvAllowListScrub {
+ private static final Logger log = LoggerFactory.getLogger(EnvAllowListScrub.class);
+
/** 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}. */
+ static final String SCRUB_FILE = "scrub.zsh";
+
+ /** Prefix of every generated directory — also what {@link #reapOrphans} matches on. */
+ static final String DIR_PREFIX = "bridged-zdotdir-";
+
+ /**
+ * How old an orphan must be before {@link #reapOrphans} removes it. Comfortably longer than any
+ * spawn takes, so a directory belonging to a pane that is still starting is never removed.
+ */
+ private static final Duration ORPHAN_AGE = Duration.ofHours(24);
+
+ /** Appended to the two startup files that must run the scrub, after their {@code $HOME} source. */
+ private static final String SOURCE_SCRUB =
+ "source \"$ZDOTDIR/" + SCRUB_FILE + "\"\n";
+
private EnvAllowListScrub() {
}
@@ -61,12 +99,17 @@ public final class EnvAllowListScrub {
*/
public static Path generate(Path parentDir, Set {@link #generate} registers each directory for deletion at JVM exit, which covers a clean
+ * shutdown and covers nothing else. A {@code kill -9}, a crash, or a host reboot leaves the
+ * directory in the temp dir for good, and the daemon is restarted often enough that these
+ * accumulate. They hold no secrets — the generated files contain variable NAMES and a report of
+ * names, never a value — but an unbounded pile of them in {@code /tmp} is still our mess to
+ * clear.
+ *
+ * Called from {@link #generate}, so it runs on the path that creates them and needs no
+ * separate wiring or scheduler. Only directories older than {@link #ORPHAN_AGE} are touched,
+ * which keeps it clear of any pane that is merely still starting, including one belonging to a
+ * different daemon instance running right now. Best-effort: every failure is ignored, because
+ * tidying temp files must never be the reason a spawn fails.
+ */
+ static void reapOrphans(Path parentDir) {
+ Instant cutoff = Instant.now().minus(ORPHAN_AGE);
+ try (Stream A missing report is a WARN, not a debug line. The report is the only evidence that
+ * the scrub ran at all in that pane. Its absence has an innocent reading — the pane died before
+ * its shell finished starting — and a serious one: the shell was not zsh, or it read its
+ * startup files from somewhere other than the directory we generated, in which case the member
+ * ran for its whole life with the operator's full secret store in its environment and nothing
+ * said so. We cannot tell those two apart from here, so the line says what is and is not known
+ * rather than picking one. Logging this at debug is how a control that silently stopped working
+ * stays unnoticed — the failure mode this whole class exists to remove.
*/
private void releaseZdotdir(String paneId) {
Path dir = zdotdirByPane.remove(paneId);
@@ -1086,7 +1107,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
}
EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(dir);
if (report == null) {
- log.debug("memberCredentials allow-list: pane {} left no scrub report", paneId);
+ 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.",
+ paneId, dir);
} else {
log.info("memberCredentials allow-list: pane {} allowed {} of {} environment variables",
paneId, report.allowed(), report.total());
diff --git a/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java b/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java
index 9e97562..8b7138a 100644
--- a/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java
+++ b/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java
@@ -41,23 +41,27 @@ public final class MemberEnvAllowList {
/**
* Names that are not credentials and that a login shell or agent binary genuinely needs.
*
- * Deliberately conservative beyond the ticket's named set: {@code ZDOTDIR} must survive or
- * every later sub-shell loses the scrub; {@code BRIDGED_MEMBER} is the daemon's own marker;
- * {@code GITEA_TOKEN}/{@code GITEA_HOST} are what {@code applyGitToken} injects by literal name;
- * the {@code ANTHROPIC_*}/{@code CLAUDE_CONFIG_DIR}/{@code OPENCODE_CONFIG} names are what the
- * adapters inject by literal name (they are also re-added per-spawn from the env map itself —
- * listing them here keeps the derived set self-contained for tests and reporting); {@code
- * JAVA_HOME} and the {@code XDG_*} roots are toolchain locations, not secrets. Everything else a
- * member needs must arrive via a profile's {@code env:}, which lands on this set automatically.
+ * Every name here is a location or a shell setting, never a credential. That rule is load
+ * bearing, and CB-633's first cut broke it: it also listed {@code ANTHROPIC_AUTH_TOKEN},
+ * {@code GITEA_TOKEN}, {@code GITEA_HOST}, {@code ANTHROPIC_BASE_URL}, {@code ANTHROPIC_MODEL},
+ * {@code CLAUDE_CONFIG_DIR}, {@code OPENCODE_CONFIG} and {@code BRIDGED_MEMBER} "because the
+ * launcher injects them". The launcher does — but only on the spawns where it actually sets
+ * them, and {@code HerdrPeerLauncher} already unions THIS spawn's env-map keys into the
+ * allow-list. So a static entry adds nothing on a spawn that injects the name, and on a spawn
+ * that does not it lets the operator's own value through under exactly the name a member reads.
+ * {@code ANTHROPIC_BASE_URL} is the sharpest case: an inherited one silently moves a member off
+ * the endpoint the profile chose.
+ *
+ * {@code ZDOTDIR} stays because it is this control's own handle — lose it and every later
+ * sub-shell loses the scrub. {@code JAVA_HOME} and the {@code XDG_*} roots are toolchain
+ * locations. Everything else a member needs must arrive via a profile's {@code env:} or the
+ * launcher's own injection, both of which land on the derived set automatically.
*/
public static final Set Why this test exists. The first version of this control put the scrub in {@code .zlogin}
+ * alone. zsh reads {@code .zlogin} only for a login shell, and herdr does not open one
+ * everywhere: measured on herdr 0.8.0, a macOS pane runs {@code -zsh} (login) while a Linux pane
+ * runs a plain {@code /usr/bin/zsh}. So the control would have passed every test on the
+ * developer's Mac and protected nothing at all on the vhost it was being built for, in silence.
+ *
+ * This runs {@code zsh -i} — no {@code -l} — so {@code .zprofile} and {@code .zlogin} are
+ * skipped exactly as they are on Linux. It therefore tests the Linux code path from a Mac,
+ * which is the only place we can currently run it. Reverting the scrub to {@code .zlogin} only
+ * makes this test fail while the login-shell test above still passes.
+ */
+ @Test
+ void scrubAlsoRunsInAnInteractiveNonLoginShell(@TempDir Path tmp) throws Exception {
+ assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
+ Path homeZshrc = Path.of(System.getProperty("user.home"), ".zshrc");
+ assumeTrue(Files.exists(homeZshrc), "$HOME/.zshrc does not exist — no real chain to test against");
+
+ Set Why this test exists, and why it is separate from {@link EnvAllowListScrubTest}. Every other
+ * test of this feature calls {@code EnvAllowListScrub} or {@code MemberEnvAllowList} itself. Those
+ * prove the scrub is correct. None of them proves anyone runs it: deleting the single
+ * {@code applyEnvironmentAllowListPolicy(cfg, launch)} line from {@code spawnInternal} left all 896
+ * tests green while turning the control completely off. That is the recurring shape in this
+ * codebase — a feature behind one call, with every test on the far side of it (CB-586, CB-611).
+ *
+ * So this test starts a real spawn through {@link HerdrPeerLauncher#spawn} and asserts on what
+ * reached herdr. It deliberately checks the pane-creation parameters rather than the launcher's own
+ * map, because the map is an intermediate: {@code ZDOTDIR} only protects anything if it is in the
+ * env herdr uses to create the pane, and that is the last point we can observe before the shell
+ * starts.
+ */
+class HerdrPeerLauncherAllowListWiringTest {
+
+ /** A name the daemon itself injects — it must survive its own scrub, so it must be allowed. */
+ private static final String INJECTED = "ANTHROPIC_BASE_URL";
+
+ @Test
+ void spawningUnderAllowListPolicyGivesThePaneAGeneratedZdotdir() {
+ FakeHerdr herdr = new FakeHerdr();
+ WiringLauncher launcher = new WiringLauncher(herdr, allowList());
+
+ launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
+
+ String paneParams = String.valueOf(herdr.lastCall("pane.split").params());
+ assertTrue(paneParams.contains("ZDOTDIR"),
+ "the spawn must hand herdr a ZDOTDIR so the pane's zsh reads our generated startup "
+ + "files; without it the scrub never runs and the member inherits the whole "
+ + "host environment. pane.split params were: " + paneParams);
+
+ Path dir = Path.of(launcher.env.get("ZDOTDIR"));
+ assertTrue(Files.isDirectory(dir), "ZDOTDIR must point at a directory that exists: " + dir);
+ // Both startup files must exist and both must source the scrub: .zlogin covers macOS panes
+ // (login shells), .zshrc covers Linux panes (interactive, NOT login). Checking only one
+ // would pass on the platform it was written for and ship a dead control on the other.
+ for (String file : List.of(".zshrc", ".zlogin")) {
+ Path f = dir.resolve(file);
+ assertTrue(Files.isRegularFile(f), file + " must be generated: " + f);
+ assertTrue(readAll(f).contains(EnvAllowListScrub.SCRUB_FILE),
+ file + " must source " + EnvAllowListScrub.SCRUB_FILE + " — a scrub only one of "
+ + "them runs is dead on the platform that reads the other");
+ }
+ assertTrue(readAll(dir.resolve(EnvAllowListScrub.SCRUB_FILE)).contains(INJECTED),
+ "the allow-list must include the names this very launch injects (" + INJECTED
+ + "), or the daemon's own configuration is blanked by its own control");
+ }
+
+ /** The default policy must not generate anything — an upgrade changes nothing until asked. */
+ @Test
+ void spawningUnderTheDefaultPolicyGeneratesNoZdotdir() {
+ FakeHerdr herdr = new FakeHerdr();
+ WiringLauncher launcher = new WiringLauncher(herdr, () -> new FleetConfig.MemberCredentials(
+ null, List.of(), List.of(), null));
+
+ launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
+
+ assertFalse(launcher.env.containsKey("ZDOTDIR"),
+ "policy=deny-by-default is the shipped default; it must not silently start "
+ + "rewriting members' shell startup files");
+ }
+
+ /**
+ * A non-zsh shell cannot read {@code ZDOTDIR} at all. The launcher must fall back rather than
+ * generate a directory nothing will ever read — a directory that would look like protection.
+ */
+ @Test
+ void aNonZshShellGeneratesNothingAndFallsBack() {
+ FakeHerdr herdr = new FakeHerdr();
+ WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/bash");
+
+ launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
+
+ assertFalse(launcher.env.containsKey("ZDOTDIR"),
+ "bash ignores ZDOTDIR; setting it would be protection theatre");
+ }
+
+ private static Supplier