From bd2774b5f1d547035ce4a33ea806056e26984163 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 13:13:23 +0700 Subject: [PATCH] fleetd #155: refuse a member spawn under memberCredentials.policy=allow-list on a non-zsh shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ZDOTDIR scrub that enforces policy=allow-list only runs on zsh. The daemon already detected a non-zsh login shell (isZshShell/warnNonZsh, from #213), but degraded to the weaker CB-596 overlay and spawned anyway — the exact "control silently does nothing" defect this ticket is about. Now a non-zsh shell under policy=allow-list refuses the spawn (IllegalArgumentException, naming the shell), surfaced by FleetMcp.spawn's existing catch(IllegalArgumentException). policy=deny-by-default is unaffected in substance (its overlay never depended on the shell) but now also logs a one-time WARN naming the shell, since the stronger allow-list control is unavailable there. The worktreeRoot/worktreeGroup-missing degrade path under memberHerdrSocket is untouched — that gap is fleetd #213's scope, not this one. --- .../dev/ltms/fleet/config/FleetConfig.java | 12 +- .../ltms/fleet/member/HerdrPeerLauncher.java | 82 ++++++++++---- .../fleet/member/ClaudeCodeLauncherTest.java | 39 +++---- .../HerdrPeerLauncherAllowListWiringTest.java | 104 +++++++++++------- 4 files changed, 148 insertions(+), 89 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index 81300ac..cd6e102 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -92,9 +92,15 @@ import java.util.regex.PatternSyntaxException; * fleetd's own process, so fleetd's own {@code $SHELL} says nothing about what * that pane runs. There is no channel to ask herdr for another user's shell, so * this must be told, never guessed. {@code null}/blank (or a value not ending - * in {@code zsh}) is treated the same as "not zsh": the {@code - * memberCredentials.policy: allow-list} ZDOTDIR scrub is skipped in favour of - * the CB-596 sentinel overlay — a degraded control, never a refusal to spawn. + * in {@code zsh}) is treated the same as "not zsh". fleetd #155: what that means + * now depends on {@code memberCredentials.policy}. Under {@code deny-by-default} + * it stays a degraded control, never a refusal to spawn — the pane-creation + * overlay is unaffected by shell type, so the spawn proceeds with a WARN naming + * the shell. Under {@code allow-list} the spawn is REFUSED instead: that policy's + * whole point is a control a sourced file cannot undo, so silently falling back + * to the weaker overlay would be the same "control silently does nothing" defect + * #155 exists to remove — configure this field (or move the account to zsh, or + * switch policy back to {@code deny-by-default}) to unblock the spawn. * When {@code memberHerdrSocket} is NOT configured this field is never * consulted at all; fleetd keeps reading its own {@code $SHELL}, exactly as * before this field existed. diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java index 3ae40d7..990b076 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -184,7 +184,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { */ private final ConcurrentMap zdotdirByPane = new ConcurrentHashMap<>(); - /** Guards {@link #warnNonZsh} to one WARN per launcher instance, not one per spawn. */ + /** + * Guards {@link #warnNonZsh} to one WARN per launcher instance, not one per spawn. fleetd #155: + * only the {@code policy: deny-by-default} path still warns on a non-zsh shell — the {@code + * policy: allow-list} path refuses the spawn instead (see {@link #applyEnvironmentAllowListPolicy}). + */ private final AtomicBoolean nonZshShellWarned = new AtomicBoolean(); /** Live config provides URI environment names that must never enter member panes. */ private final Supplier config; @@ -1255,6 +1259,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { if (!creds.isAllowList()) { overlayBlockedCredentials(workerEnv, creds); logCredentialGap(creds, null); + // fleetd #155: this overlay itself does not depend on the shell (it lands in the + // pane-creation env map before any shell runs), so the spawn is never refused here — + // only policy=allow-list's ZDOTDIR scrub needs a login shell to run at all. Still worth + // telling the operator: the stronger post-shell control is unavailable on this shell. + warnNonZsh(memberLoginShell()); } } @@ -1307,10 +1316,16 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * dev.ltms.fleet.session.Worktrees#shareWithGroup} already uses, reused rather than * inventing a second group key. Either one missing means the scrub cannot be guaranteed * reachable by the member, which is the same "cannot guarantee the scrub runs" case as a - * non-zsh shell, so it gets the identical fallback. + * non-zsh shell — that one still gets the fallback (see fleetd #155's javadoc note below + * on why a missing worktreeRoot/worktreeGroup is a different defect, out of that ticket's + * scope). * - * In every branch: never refuse to spawn. A degraded credential control must not become an - * outage for an opt-in feature. + * fleetd #155: the one exception to "never refuse to spawn" is a non-zsh login shell, right + * below. The operator asked for {@code policy: allow-list} specifically — its whole point is a + * control a sourced file cannot undo — so silently degrading to the weaker overlay is exactly + * the "silently does nothing" failure this ticket exists to remove. Every other branch in this + * method (worktreeRoot/worktreeGroup missing) keeps the old "degrade, never refuse" behaviour; + * that gap is real but is fleetd #213's scope, not this one. */ private Path applyEnvironmentAllowListPolicy(FleetConfig.Profile cfg, Launch launch) { FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get(); @@ -1324,20 +1339,21 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { // must not even be called on this path, only the explicit memberLoginShell: config can // answer it. With memberHerdrSocket absent, nothing here changes: fleetd's own $SHELL is // still the input, exactly as before this fix. - String loginShell = memberHerdrSocket ? configuredMemberLoginShell() : resolveEnv("SHELL"); + String loginShell = memberLoginShell(); boolean zsh = isZshShell(loginShell); if (!zsh) { - // A non-zsh login shell ignores ZDOTDIR entirely: NO scrub would run, so pretending - // otherwise would be worse than saying so. Warn loudly and fall back to the CB-596 - // sentinel overlay over the enumerated known: names — weaker (a sourced file can undo - // it), but strictly better than nothing. Deliberately no "allowed N of M" line here: the - // scrub this count describes does not run on this path, so printing it would tell an - // operator that a fraction of names were blocked when the real number blocked is zero. - // logCredentialGap's WARN (below) is the only signal for this path. - warnNonZsh(loginShell); - overlayBlockedCredentials(launch.env(), creds); - logCredentialGap(creds, null); - return null; + // fleetd #155: a non-zsh login shell ignores ZDOTDIR entirely — NO scrub would run. The + // operator explicitly asked for policy=allow-list's blocking control, so degrading to + // the weaker CB-596 overlay and spawning anyway would be the same "control silently does + // nothing" defect this ticket exists to close. Refuse instead — the caller (FleetMcp.spawn) + // catches IllegalArgumentException and surfaces the message to the operator. + throw new IllegalArgumentException( + "memberCredentials policy=allow-list requires the member's login shell to be " + + "zsh, so the ZDOTDIR scrub can run after it — refusing to spawn under " + + "login shell '" + (loginShell == null ? "" : loginShell) + + "'. Configure memberLoginShell: as a zsh path (only read when " + + "memberHerdrSocket is set), move the member's OS account onto zsh, or " + + "set memberCredentials.policy: deny-by-default instead."); } Path parentDir; String group = null; @@ -1392,6 +1408,20 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { return cfg == null ? null : cfg.memberLoginShell(); } + /** + * fleetd #155: the member pane's login shell, using the same {@code memberHerdrSocket} routing + * {@link #applyEnvironmentAllowListPolicy} already used — fleetd's own {@code $SHELL} decides it + * when {@code memberHerdrSocket} is absent (today's only mode); the configured {@code + * memberLoginShell:} decides it otherwise, since fleetd's own {@code $SHELL} names a different + * user's shell once member panes run under a different OS user. Shared by both {@link + * #applyEnvironmentAllowListPolicy} (allow-list: refuses on non-zsh) and {@link + * #applyMemberCredentialPolicy} (deny-by-default: warns on non-zsh) so the two policies agree on + * what "the member's shell" means. + */ + private String memberLoginShell() { + return memberHerdrSocketConfigured() ? configuredMemberLoginShell() : resolveEnv("SHELL"); + } + /** * fleetd #213: {@code worktreeRoot}, as the ZDOTDIR scrub's parent directory under {@code * memberHerdrSocket}, or {@code null} when unconfigured — the same "cannot guarantee the scrub @@ -1476,15 +1506,23 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { private static final String SSH_AUTH_SOCK = MemberEnvAllowList.SSH_AUTH_SOCK; /** - * CB-633: a non-zsh login shell means the allow-list control CANNOT run — say so once per - * launcher instance, naming the shell, instead of failing silently. + * fleetd #155: called ONLY from the {@code policy: deny-by-default} branch of {@link + * #applyMemberCredentialPolicy} — under {@code policy: allow-list}, a non-zsh shell now REFUSES + * the spawn instead (see {@link #applyEnvironmentAllowListPolicy}), since that policy's whole + * point is a control a sourced file cannot undo. deny-by-default's own overlay does not depend + * on the shell (it lands in the pane-creation env map before any shell runs), so this is a + * heads-up, not a defect report: say once per launcher instance, naming the shell, that the + * stronger allow-list control is unavailable here — never silently. */ private void warnNonZsh(String shell) { + if (isZshShell(shell)) { + return; + } if (nonZshShellWarned.compareAndSet(false, true)) { - log.warn("memberCredentials policy=allow-list: member login shell '{}' is NOT zsh — " - + "ZDOTDIR scrubbing cannot run, so members' inherited environment is " - + "UNPROTECTED beyond the enumerated known: fallback. Move herdr onto a " - + "zsh account or switch policy back to deny-by-default.", + log.warn("memberCredentials policy=deny-by-default: member login shell '{}' is NOT zsh — " + + "the pane-creation credential overlay still applies here (it does not " + + "depend on the shell), but policy=allow-list's stronger post-shell " + + "ZDOTDIR scrub is unavailable on this shell.", shell == null ? "" : shell); } } diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java index d995775..64b5101 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java @@ -1409,14 +1409,16 @@ class ClaudeCodeLauncherTest { } /** - * CB-633 follow-up (#192): the mirror of the zsh test above. On a non-zsh login shell {@code - * ZDOTDIR} is ignored, so no scrub ever runs — the report must keep the WARN wording (a name here - * really is inherited unblocked) rather than claiming a scrub protects it. This is the trap PR - * #174 fell into the other direction: keying the wording on the shell, not on {@code - * creds.isAllowList()}, is what keeps this branch correct. + * fleetd #155 (was CB-633 follow-up (#192) "allowListPolicyOnNonZshKeepsTheWarnWording"): on a + * non-zsh login shell {@code ZDOTDIR} is ignored, so no scrub could ever run there. Before #155 + * the launcher degraded to the weaker overlay and spawned anyway; that is exactly the "control + * silently does nothing" failure #155 exists to close, since {@code policy: allow-list} is the + * operator explicitly asking for a control a sourced file cannot undo. Now the spawn is REFUSED + * instead — this test asserts the refusal, naming the shell, on the real spawn path ({@link + * ClaudeCodeLauncher#spawn}), not merely on the launcher method that computes it. */ @Test - void allowListPolicyOnNonZshKeepsTheWarnWording() { + void allowListPolicyOnNonZshRefusesTheSpawn() { FakeHerdr herdr = new FakeHerdr(); FleetConfig.Profile cfg = new FleetConfig.Profile( "ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", @@ -1430,27 +1432,12 @@ class ClaudeCodeLauncherTest { 0, System::currentTimeMillis, () -> {}, null, () -> creds, () -> Set.of("AI_GATEWAY_TOKEN", "A_BRAND_NEW_SECRET_TOKEN")); - Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class); - ListAppender appender = new ListAppender<>(); - appender.start(); - logger.addAppender(appender); - try { - svc.spawn(); - } finally { - logger.detachAppender(appender); - } + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, svc::spawn, + "policy=allow-list on a non-zsh shell must refuse the spawn, not silently degrade " + + "to the weaker overlay"); - assertTrue(appender.list.stream().anyMatch(e -> - e.getFormattedMessage().contains("memberCredentials gap") - && e.getFormattedMessage().contains("UNBLOCKED") - && e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN")), - "no scrub runs on a non-zsh shell, so the WARN wording must be kept — got: " - + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList()); - assertFalse(appender.list.stream().anyMatch(e -> - e.getFormattedMessage().contains("memberCredentials gap") - && e.getFormattedMessage().toLowerCase(java.util.Locale.ROOT).contains("scrub")), - "nothing is scrubbed on this path, so the report must not claim otherwise — got: " - + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList()); + assertTrue(e.getMessage().contains("allow-list") && e.getMessage().contains("/bin/bash"), + "the refusal must name the policy and the actual shell, got: " + e.getMessage()); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java index 2d0d69b..75f883c 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -29,6 +29,7 @@ import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -99,18 +100,26 @@ class HerdrPeerLauncherAllowListWiringTest { } /** - * 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. + * fleetd #155: a non-zsh shell cannot read {@code ZDOTDIR} at all, so under {@code + * policy: allow-list} — the policy the operator picked specifically for a control a sourced file + * cannot undo — the launcher must REFUSE the spawn rather than silently generate a directory + * nothing will ever read (protection theatre) or fall back to the weaker overlay (exactly the + * "control silently does nothing" defect this ticket exists to close). Real path: through {@link + * HerdrPeerLauncher#spawn}, the method the daemon actually calls. */ @Test - void aNonZshShellGeneratesNothingAndFallsBack() { + void aNonZshShellUnderAllowListPolicyRefusesTheSpawn() { FakeHerdr herdr = new FakeHerdr(); WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/bash"); - launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)), + "policy=allow-list on a non-zsh shell must refuse the spawn, not silently degrade"); + assertTrue(e.getMessage().contains("allow-list") && e.getMessage().contains("/bin/bash"), + "the refusal must name the policy and the actual shell, got: " + e.getMessage()); assertFalse(launcher.env.containsKey("ZDOTDIR"), - "bash ignores ZDOTDIR; setting it would be protection theatre"); + "a refused spawn must not have generated (or wired in) a scrub directory: " + launcher.env); } private static Supplier allowList() { @@ -246,11 +255,14 @@ class HerdrPeerLauncherAllowListWiringTest { * "allowed N of M" line — which describes what the scrub does — must not be printed there either. * Before this fix the line was logged BEFORE the zsh gate, so a non-zsh host printed e.g. * "allowed 1 of 3" while blocking nothing at all, telling an operator a control ran when it did - * not. Real path: goes through {@link HerdrPeerLauncher#spawn}, same as the sibling test above, - * with the shell fixed to bash so the fallback branch is the one exercised. + * not. fleetd #155: the spawn itself is now refused on this path (see {@code + * aNonZshShellUnderAllowListPolicyRefusesTheSpawn}) rather than falling back — this test's own + * concern still holds under the refusal: the "allowed N of M" line describes a scrub that never + * ran here, so it must still never appear. Real path: goes through {@link + * HerdrPeerLauncher#spawn}, same as the sibling test above, with the shell fixed to bash. */ @Test - void noAllowedCountLineIsEmittedOnTheNonZshFallbackPath() { + void noAllowedCountLineIsEmittedOnTheNonZshRefusalPath() { FakeHerdr herdr = new FakeHerdr(); Set hostEnvNames = Set.of(INJECTED, "SOME_UNRELATED_NAME", "ANOTHER_UNRELATED_NAME"); WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/bash", () -> hostEnvNames); @@ -262,7 +274,9 @@ class HerdrPeerLauncherAllowListWiringTest { appender.start(); logger.addAppender(appender); try { - launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + assertThrows(IllegalArgumentException.class, + () -> launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)), + "policy=allow-list on a non-zsh shell must refuse the spawn"); } finally { logger.detachAppender(appender); logger.setLevel(original); @@ -310,13 +324,20 @@ class HerdrPeerLauncherAllowListWiringTest { * different OS user — {@link HerdrPeerLauncher#hostEnvNames} describes fleetd's own process, not * that user's. Neither "inherits them UNBLOCKED" nor "scrub blanks them" is evidence-backed * there, so neither may print; the single unknown-environment WARN must, naming the config key. + * + *

fleetd #155: {@code memberLoginShell} is now given explicitly as zsh, so the spawn reaches + * this WARN through the (still-degrading, not refusing) missing-{@code worktreeRoot}/{@code + * worktreeGroup} fallback rather than through the zsh gate, which now refuses instead of falling + * back — see {@code aNonZshShellUnderAllowListPolicyRefusesTheSpawn}. This test's own concern + * (the unknown-environment WARN) is orthogonal to which fallback reached {@code + * logCredentialGap}, so it still holds. */ @Test void gapDetectorReportsUnknownInsteadOfAConclusionWhenMemberHerdrSocketIsConfigured() { FakeHerdr herdr = new FakeHerdr(); Set hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN"); WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames, - () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock")); + () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", "/bin/zsh")); List messages = spawnAndCaptureLogs(launcher); @@ -340,8 +361,9 @@ class HerdrPeerLauncherAllowListWiringTest { void theUnknownEnvironmentWarnFiresOnceNotOncePerSpawn() { FakeHerdr herdr = new FakeHerdr(); Set hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN"); + // fleetd #155: memberLoginShell given explicitly as zsh — see the sibling test above for why. WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames, - () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock")); + () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", "/bin/zsh")); Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class); Level original = logger.getLevel(); @@ -389,42 +411,44 @@ class HerdrPeerLauncherAllowListWiringTest { } /** - * fleetd #213 defect 1, acceptance criterion 1: {@code memberHerdrSocket} configured and {@code - * memberLoginShell} configured as non-zsh must fall back to the sentinel overlay exactly like a - * non-zsh {@code $SHELL} does today — and the "generated ZDOTDIR" INFO must not appear, since no - * scrub actually runs. The WiringLauncher's own {@code env("SHELL")} is deliberately set to - * {@code /bin/zsh} — the OPPOSITE of what {@code memberLoginShell} says — so a launcher that - * (incorrectly) fell back to fleetd's own {@code $SHELL} here would wrongly pass the gate and - * fail this test. + * fleetd #213 defect 1, acceptance criterion 1 — updated by fleetd #155: {@code + * memberHerdrSocket} configured and {@code memberLoginShell} configured as non-zsh must now + * REFUSE the spawn (not fall back to the sentinel overlay — see {@code + * aNonZshShellUnderAllowListPolicyRefusesTheSpawn}'s javadoc for why). The WiringLauncher's own + * {@code env("SHELL")} is deliberately set to {@code /bin/zsh} — the OPPOSITE of what {@code + * memberLoginShell} says — so a launcher that (incorrectly) fell back to fleetd's own {@code + * $SHELL} here would wrongly pass the gate and fail this test. */ @Test - void memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay() { + void memberHerdrSocketWithNonZshMemberLoginShellRefusesTheSpawn() { FakeHerdr herdr = new FakeHerdr(); WiringLauncher launcher = new WiringLauncher(herdr, allowListWithKnown(List.of("SOME_TOKEN")), "/bin/zsh", null, () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", "/bin/bash")); - List messages = spawnAndCaptureLogs(launcher); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)), + "a non-zsh configured memberLoginShell under policy=allow-list must refuse the spawn"); - assertEquals("blocked-by-fleetd-cb596-see-gitea-issue-82", launcher.env.get("SOME_TOKEN"), - "a non-zsh memberLoginShell must fall back to the CB-596 sentinel overlay, exactly " - + "like a non-zsh $SHELL does when memberHerdrSocket is absent"); + assertTrue(e.getMessage().contains("/bin/bash"), + "the refusal must name the configured memberLoginShell, got: " + e.getMessage()); + assertFalse(launcher.env.containsKey("SOME_TOKEN"), + "a refused spawn must not have touched the pane env at all: " + launcher.env); assertFalse(launcher.env.containsKey("ZDOTDIR"), "no scrub directory may be generated when the configured member login shell is not zsh"); - assertFalse(messages.stream().anyMatch(m -> m.contains("generated ZDOTDIR")), - "the 'generated ZDOTDIR' INFO must not appear when the scrub never runs — got: " + messages); } /** - * fleetd #213 defect 1, acceptance criterion 2: {@code memberHerdrSocket} configured and NO - * {@code memberLoginShell} configured must fall back exactly like criterion 1 above — AND - * fleetd's own {@code $SHELL} must never even be consulted (not merely "not decisive"). The - * fixture's {@code env} function reports {@code /bin/zsh} for {@code SHELL} — a value that would - * WRONGLY pass the zsh gate if the fix regressed to reading it — while flagging whether it was - * ever asked for at all, so this test fails loudly on either kind of regression. + * fleetd #213 defect 1, acceptance criterion 2 — updated by fleetd #155: {@code + * memberHerdrSocket} configured and NO {@code memberLoginShell} configured must now REFUSE the + * spawn exactly like criterion 1 above — AND fleetd's own {@code $SHELL} must never even be + * consulted (not merely "not decisive"). The fixture's {@code env} function reports {@code + * /bin/zsh} for {@code SHELL} — a value that would WRONGLY pass the zsh gate if the fix + * regressed to reading it — while flagging whether it was ever asked for at all, so this test + * fails loudly on either kind of regression. */ @Test - void memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell() { + void memberHerdrSocketWithNoMemberLoginShellRefusesAndNeverConsultsFleetdsOwnShell() { FakeHerdr herdr = new FakeHerdr(); AtomicBoolean shellQueried = new AtomicBoolean(false); Function env = name -> { @@ -437,15 +461,18 @@ class HerdrPeerLauncherAllowListWiringTest { WiringLauncher launcher = new WiringLauncher(herdr, allowListWithKnown(List.of("SOME_TOKEN")), env, () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", null)); - List messages = spawnAndCaptureLogs(launcher); + assertThrows(IllegalArgumentException.class, + () -> launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)), + "no configured memberLoginShell under policy=allow-list must refuse the spawn, same " + + "as an explicit non-zsh one"); assertFalse(shellQueried.get(), "fleetd's own $SHELL must never be consulted once " + "memberHerdrSocket is configured — only memberLoginShell: may decide the gate"); - assertEquals("blocked-by-fleetd-cb596-see-gitea-issue-82", launcher.env.get("SOME_TOKEN"), - "no memberLoginShell configured must fall back to the sentinel overlay, same as a " + assertFalse(launcher.env.containsKey("SOME_TOKEN"), + "a refused spawn must not have touched the pane env at all, same as a " + "configured non-zsh shell"); - assertFalse(messages.stream().anyMatch(m -> m.contains("generated ZDOTDIR")), - "no scrub may run without a configured memberLoginShell — got: " + messages); + assertFalse(launcher.env.containsKey("ZDOTDIR"), + "no scrub may run without a configured memberLoginShell"); } /** @@ -700,7 +727,8 @@ class HerdrPeerLauncherAllowListWiringTest { /** * fleetd #213: as above, plus {@code worktreeRoot:}/{@code worktreeGroup:} — both required for * the ZDOTDIR scrub to run at all once {@code memberHerdrSocket} is configured; either missing - * falls back to the sentinel overlay, same as a non-zsh {@code memberLoginShell}. + * falls back to the sentinel overlay (fleetd #155 left this branch alone — it is not a shell + * problem, so it is not this ticket's refusal). */ private static FleetConfig configWithMemberHerdrSocketRootAndGroup(String memberHerdrSocket, String memberLoginShell, String worktreeRoot, String worktreeGroup) {