diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index ad4641a..80a9707 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -117,6 +117,15 @@ herdrSocket: ~/.config/herdr/herdr.sock # Optional socket for member panes. Omit this to use herdrSocket for both leads and members. # memberHerdrSocket: /Users/member/.config/herdr/herdr.sock +# fleetd #213: the login shell the member OS user (memberHerdrSocket above) actually runs. ONLY +# read when memberHerdrSocket is set — fleetd's own $SHELL says nothing about a pane running +# under a different OS user, and there is no channel to ask herdr for that user's shell, so this +# must be told rather than guessed. Absent, blank, or anything not ending in "zsh" is treated the +# same as "not zsh": the memberCredentials.policy: allow-list ZDOTDIR scrub (see worktreeGroup +# below) is skipped in favour of the weaker CB-596 sentinel overlay — a degraded control, never a +# refusal to spawn. When memberHerdrSocket is absent this key is never consulted at all. +# memberLoginShell: /bin/zsh + # How member sessions are spawned. Define one or more named profiles (backends) under # `profiles`; each key is the profile name (also the ccs profile). A profile says only WHICH # BACKEND — model, CLI adapter, credentials, cost. It says nothing about what a member spawned on @@ -643,6 +652,13 @@ guard: # CAUTION: this isolates credentials, not the repository — a member in the group can still # write the operator's git objects and refs in the shared repo. The operator running fleetd # must already be a member of the named group, or every provisioning spawn fails loudly. +# +# fleetd #213: this is also the ONE group the memberCredentials.policy: allow-list ZDOTDIR scrub +# reuses when memberHerdrSocket is set — deliberately not a second config key. Under +# memberHerdrSocket, the scrub directory is generated under worktreeRoot (never java.io.tmpdir, +# which the member OS user cannot reach) and shared read-only with this group. If worktreeGroup +# is unset while memberHerdrSocket is set, the scrub cannot be guaranteed reachable by the member, +# so fleetd falls back to the weaker CB-596 sentinel overlay instead (a WARN names the gap). # worktreeGroup: fleet-workers # Session lifecycle limits (CB-303). All knobs are opt-in; omit or set to null to keep 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 8793c44..27ed389 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -84,6 +84,18 @@ import java.util.Set; * This isolates credentials, not the repository: a member in * the group can still write the operator's git objects and refs in the shared * repo. See {@link dev.ltms.fleet.session.Worktrees#shareWithGroup}. + * @param memberLoginShell fleetd #213: the login shell the member's OS user actually runs, ONLY + * meaningful (and only ever read) when {@code memberHerdrSocket} is + * configured — that mode spawns member panes under a different OS user than + * 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. + * 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. */ @JsonIgnoreProperties(ignoreUnknown = true) public record FleetConfig( @@ -107,7 +119,20 @@ public record FleetConfig( Integer quarantineCooldownSeconds, MemberCredentials memberCredentials, Coordinator coordinator, - String worktreeGroup) { + String worktreeGroup, + String memberLoginShell) { + + /** Back-compat form before the {@code memberLoginShell} key was added. */ + public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map profiles, + Guard guard, String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs, + Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet, + LeadHeartbeat leadHeartbeat, Health health, String placement, Auth auth, + ConfigReload configReload, Integer quarantineCooldownSeconds, + MemberCredentials memberCredentials, Coordinator coordinator, String worktreeGroup) { + this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, + spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth, + configReload, quarantineCooldownSeconds, memberCredentials, coordinator, worktreeGroup, null); + } /** Back-compat form before the {@code worktreeGroup} key was added. */ public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map profiles, @@ -118,7 +143,7 @@ public record FleetConfig( MemberCredentials memberCredentials, Coordinator coordinator) { this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth, - configReload, quarantineCooldownSeconds, memberCredentials, coordinator, null); + configReload, quarantineCooldownSeconds, memberCredentials, coordinator, null, null); } /** Back-compat form before the {@code coordinator:} block was added. */ @@ -1346,7 +1371,7 @@ public record FleetConfig( "bind", "herdrSocket", "memberHerdrSocket", "profiles", "guard", "worktreeRoot", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet", "leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds", - "memberCredentials", "coordinator", "worktreeGroup"); + "memberCredentials", "coordinator", "worktreeGroup", "memberLoginShell"); /** Load and validate config from {@code path}. */ public static FleetConfig load(Path path) { @@ -1966,9 +1991,12 @@ public record FleetConfig( // and this ticket's Coordinator is config-only anyway (nothing yet reads it at startup). // worktreeGroup is left as-is (fleetd #185 stage 3): null/blank is "off", and there is no // sane non-null default — an OS group name is operator-specific. + // memberLoginShell is left as-is (fleetd #213), like worktreeGroup: null/blank is "not + // configured", and there is no sane non-null default — a member's login shell is + // operator-specific and only meaningful when memberHerdrSocket is also set. return new FleetConfig(b, herdrSocket, memberHerdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs, broker, primary, f, leadHeartbeat, health, placementOrDefault, a, configReload, - quarantineCooldown, mc, coordinator, worktreeGroup); + quarantineCooldown, mc, coordinator, worktreeGroup, memberLoginShell); } /** 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 dee7134..6480720 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -7,6 +7,9 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermissions; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -116,6 +119,70 @@ public final class EnvAllowListScrub { } } + /** + * fleetd #213: as {@link #generate(Path, Set)}, plus share the generated directory with + * {@code group} — the member OS user's group (the operator's existing {@code worktreeGroup:} + * name, reused rather than inventing a second one) — so a member running under a different OS + * user than fleetd's own process can still read what it needs from a directory placed outside + * {@code java.io.tmpdir}. {@code group} null/blank ⇒ identical to {@link #generate(Path, Set)}; + * this is the single-daemon (no {@code memberHerdrSocket}) shape, where the pane is fleetd's own + * uid and no group sharing is needed. + * + * @throws UncheckedIOException also when {@code group} does not resolve on this host, or a + * group-ownership/permission call is refused — the same "fail + * loudly rather than start unprotected" contract as above: a scrub + * the configured member user cannot even read is not a working + * control. + */ + public static Path generate(Path parentDir, Set allowedNames, String group) { + Path dir = generate(parentDir, allowedNames); + if (group != null && !group.isBlank()) { + shareWithGroup(dir, group); + } + return dir; + } + + /** + * chgrp/chmod-equivalent over the freshly generated directory and the startup files already + * written into it: owner keeps full access, {@code group} gets traverse+read on the directory + * ({@code rwxr-x---}, so a login shell under that group can find and source the files) and + * read-only on each file ({@code rw-r-----}) — deliberately no group WRITE anywhere, since a + * member never needs to add or change fleetd's own generated scrub. (The scrub script's own + * report write inside the pane consequently fails closed rather than open — see {@code + * scrub.zsh}'s trailing {@code 2>/dev/null} — which {@link + * dev.ltms.fleet.member.HerdrPeerLauncher#releaseZdotdir} already treats as "cannot be + * confirmed to have run" rather than success.) + */ + private static void shareWithGroup(Path dir, String group) { + try { + GroupPrincipal principal = dir.getFileSystem().getUserPrincipalLookupService() + .lookupPrincipalByGroupName(group); + setGroupAndPermissions(dir, principal, "rwxr-x---"); + try (Stream entries = Files.list(dir)) { + for (Path file : entries.toList()) { + setGroupAndPermissions(file, principal, "rw-r-----"); + } + } + } catch (IOException e) { + throw new UncheckedIOException("cannot share generated ZDOTDIR " + dir + " with group '" + + group + "' — the group must exist, and the fleetd operator (" + + System.getProperty("user.name") + ") must be a member of it", e); + } catch (UnsupportedOperationException e) { + throw new UncheckedIOException("cannot share generated ZDOTDIR " + dir + " with group '" + + group + "' — this filesystem does not support POSIX group ownership", + new IOException(e)); + } + } + + private static void setGroupAndPermissions(Path path, GroupPrincipal group, String perms) throws IOException { + PosixFileAttributeView view = Files.getFileAttributeView(path, PosixFileAttributeView.class); + if (view == null) { + throw new IOException("POSIX file attributes are not supported for " + path); + } + view.setGroup(group); + Files.setPosixFilePermissions(path, PosixFilePermissions.fromString(perms)); + } + /** One operator-sourcing startup file: source the {@code $HOME} counterpart, change nothing else. */ private static String homeSourcingFile(String name) { return """ 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 97b16d7..63053a6 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -1077,6 +1077,29 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * name. It stays a one-off decision because it is a live handle to the operator's ssh-agent, not * a value — a member holding it can sign with every key the agent holds, so letting it ride in * on the generic {@code allow:} list would hand that out for an unrelated reason. + * + *

fleetd #213: which shell decides the zsh gate, and where the generated directory lives, + * both depend on whether {@code memberHerdrSocket:} is configured — see {@link + * #memberHerdrSocketConfigured()}'s javadoc for why fleetd's own {@code $SHELL} and {@code + * java.io.tmpdir} describe the wrong process once member panes run under a different OS user. + *

    + *
  • {@code memberHerdrSocket} ABSENT (today's only mode): byte-identical to before this + * fix — fleetd's own {@code $SHELL} decides zsh, and the directory is generated under + * {@code java.io.tmpdir}.
  • + *
  • {@code memberHerdrSocket} PRESENT: the configured {@code memberLoginShell:} decides + * zsh instead — fleetd's own {@code $SHELL} is never consulted, since it names a + * different user's shell, not the member's. Absent/non-zsh falls back exactly like the + * non-zsh case below. When it IS zsh, the directory still cannot go under {@code + * java.io.tmpdir} (mode 0700, unreadable by another uid — the exact gap fleetd #213 + * exists to close), so it is generated under {@code worktreeRoot} instead and shared + * read-only with {@code worktreeGroup} — the same group {@link + * 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.
  • + *
+ * In every branch: never refuse to spawn. A degraded credential control must not become an + * outage for an opt-in feature. */ private Path applyEnvironmentAllowListPolicy(FleetConfig.Profile cfg, Launch launch) { FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get(); @@ -1084,8 +1107,14 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { return null; } Set allowed = derivedAllowedNames(creds, launch); - String loginShell = resolveEnv("SHELL"); - boolean zsh = loginShell != null && (loginShell.endsWith("/zsh") || loginShell.equals("zsh")); + boolean memberHerdrSocket = memberHerdrSocketConfigured(); + // fleetd #213 defect 1: under memberHerdrSocket the member pane runs as a DIFFERENT OS + // user, so fleetd's own $SHELL says nothing about what that pane runs — resolveEnv("SHELL") + // 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"); + 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 @@ -1099,13 +1128,35 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { logCredentialGap(creds, null); return null; } + Path parentDir; + String group = null; + if (memberHerdrSocket) { + // fleetd #213 defect 2: java.io.tmpdir is fleetd's own per-user temp dir (mode 0700 on + // macOS) — a member running as a different uid cannot even traverse it, let alone read + // the generated files. worktreeRoot is the only configured location a different-uid + // member can be given access to, and only WITH worktreeGroup to grant that access — + // absent either, the scrub cannot be guaranteed reachable, so this falls back exactly + // like the non-zsh case above rather than generating a directory nothing can read. + parentDir = memberScrubParentDir(); + group = memberGroup(); + if (parentDir == null || group == null) { + warnCannotShareScrubDirectory(); + overlayBlockedCredentials(launch.env(), creds); + logCredentialGap(creds, null); + return null; + } + } else { + parentDir = Path.of(System.getProperty("java.io.tmpdir")); + } // Only reached when the scrub is actually about to run — the count below describes that // scrub, so it must not be logged before this gate (see the non-zsh branch above). Same // reasoning gates logCredentialGap's wording: passing the derived `allowed` set (non-null) // here, and ONLY here, is what tells it the scrub will really blank an unkept name — #192. logAllowListCoverage(allowed); logCredentialGap(creds, allowed); - Path dir = EnvAllowListScrub.generate(Path.of(System.getProperty("java.io.tmpdir")), allowed); + Path dir = memberHerdrSocket + ? EnvAllowListScrub.generate(parentDir, allowed, group) + : EnvAllowListScrub.generate(parentDir, allowed); launch.env().put("ZDOTDIR", dir.toAbsolutePath().toString()); log.info("memberCredentials policy=allow-list: profile={} generated ZDOTDIR {} — derived " + "allow-list holds {} name(s); the pane reports allowed N of M at release", @@ -1113,6 +1164,55 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { return dir; } + /** True when {@code shell} is a zsh login shell path or bare name — the ZDOTDIR gate. */ + private static boolean isZshShell(String shell) { + return shell != null && (shell.endsWith("/zsh") || shell.equals("zsh")); + } + + /** + * fleetd #213: the configured {@code memberLoginShell:}, or {@code null} when unconfigured. + * Called ONLY from the {@code memberHerdrSocket}-configured branch of {@link + * #applyEnvironmentAllowListPolicy} — fleetd's own {@code $SHELL} is never read on that path. + * {@link #config} being {@code null} (an older test call site, or a launcher that never + * threaded the full config through) is treated the same as "not configured". + */ + private String configuredMemberLoginShell() { + FleetConfig cfg = config == null ? null : config.get(); + return cfg == null ? null : cfg.memberLoginShell(); + } + + /** + * 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 + * runs" gap as {@link #memberGroup()} being unset (see {@link + * #applyEnvironmentAllowListPolicy}). Deliberately no sibling-of-repo-root default here, unlike + * {@code GitWorktrees}' own {@code worktreeRoot} resolution: that default is a convenience for + * provisioning a worktree that will exist regardless, whereas an unconfigured value here means + * fleetd has no operator-endorsed location to put a credential-bearing directory a different OS + * user must reach, so falling back to the overlay is the honest answer, not a guess. + */ + private Path memberScrubParentDir() { + FleetConfig cfg = config == null ? null : config.get(); + if (cfg == null || cfg.worktreeRoot() == null || cfg.worktreeRoot().isBlank()) { + return null; + } + return Path.of(cfg.worktreeRoot()); + } + + /** + * fleetd #213: the configured {@code worktreeGroup:}, or {@code null} when unset/blank. Reuses + * the group {@link dev.ltms.fleet.session.Worktrees#shareWithGroup} already establishes for + * provisioned worktrees, rather than a second group key — see {@link + * #applyEnvironmentAllowListPolicy}. + */ + private String memberGroup() { + FleetConfig cfg = config == null ? null : config.get(); + if (cfg == null || cfg.worktreeGroup() == null || cfg.worktreeGroup().isBlank()) { + return null; + } + return cfg.worktreeGroup(); + } + /** * The full kept-name set for this spawn: the profile-derived names, unioned with {@code * memberCredentials.allow:} (CB-633 follow-up — previously ignored by this whole policy), the @@ -1170,6 +1270,30 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { } } + /** Guards {@link #warnCannotShareScrubDirectory} to one WARN per launcher instance. */ + private final AtomicBoolean cannotShareScrubDirWarned = new AtomicBoolean(); + + /** + * fleetd #213: {@code memberHerdrSocket} is configured and the member login shell IS zsh, but + * {@code worktreeRoot} and/or {@code worktreeGroup} is missing, so the generated ZDOTDIR cannot + * be placed anywhere the member's OS user can reach — {@code java.io.tmpdir} is fleetd's own + * 0700 temp dir, unreadable by another uid, which is the exact gap this ticket exists to close. + * Say so once per launcher instance, instead of either generating a directory nothing can read + * (protection theatre) or refusing to spawn (turning a degraded credential control into an + * outage for an opt-in feature). + */ + private void warnCannotShareScrubDirectory() { + if (cannotShareScrubDirWarned.compareAndSet(false, true)) { + log.warn("memberCredentials policy=allow-list: memberHerdrSocket is configured and the " + + "member login shell is zsh, but worktreeRoot and/or worktreeGroup is not " + + "configured — the generated ZDOTDIR cannot be placed where the member's OS " + + "user can read it (java.io.tmpdir is fleetd's own, unreadable by another uid), " + + "so the scrub cannot be guaranteed to run. Falling back to the CB-596 sentinel " + + "overlay. Configure both worktreeRoot and worktreeGroup to enable the " + + "allow-list scrub under memberHerdrSocket."); + } + } + /** * CB-633 teardown half: read the pane's scrub report (the denominator report the generated * scrub wrote) and delete the directory. Called from {@link #stop}, which is the one funnel 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 2a66b7e..cb0ea16 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -12,20 +12,26 @@ import dev.ltms.fleet.peer.Capability; import dev.ltms.fleet.peer.MemberRole; import dev.ltms.fleet.peer.SpawnRequest; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFileAttributeView; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; 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.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * CB-633: proves the allow-list scrub is actually WIRED INTO the spawn path — not merely that its @@ -119,6 +125,18 @@ class HerdrPeerLauncherAllowListWiringTest { FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, allow, List.of(), null); } + /** + * Same as {@link #allowList()} but with an operator-configured {@code known:} list, so the + * fallback overlay (the non-zsh / no-memberLoginShell path) has something visible to shadow — + * {@link FleetConfig.MemberCredentials#blockedSet()} is {@code known - allow}, so an empty + * {@code known} (what {@link #allowList()} uses) blocks nothing and a fallback test would have + * no sentinel entry to assert on. + */ + private static Supplier allowListWithKnown(List known) { + return () -> new FleetConfig.MemberCredentials( + FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), known, null); + } + /** * CB-633 follow-up: a name that lives ONLY in {@code memberCredentials.allow:} — no profile * mentions it — must survive the scrub the real spawn path generates. Calling {@code @@ -371,6 +389,137 @@ class HerdrPeerLauncherAllowListWiringTest { "the log must never contain an env var VALUE, only its NAME — got: " + messages); } + /** + * 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. + */ + @Test + void memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay() { + 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); + + 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"); + 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. + */ + @Test + void memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell() { + FakeHerdr herdr = new FakeHerdr(); + AtomicBoolean shellQueried = new AtomicBoolean(false); + Function env = name -> { + if ("SHELL".equals(name)) { + shellQueried.set(true); + return "/bin/zsh"; // would wrongly pass the zsh gate if this ever leaked through + } + return null; + }; + WiringLauncher launcher = new WiringLauncher(herdr, allowListWithKnown(List.of("SOME_TOKEN")), env, + () -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", null)); + + List messages = spawnAndCaptureLogs(launcher); + + 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 " + + "configured non-zsh shell"); + assertFalse(messages.stream().anyMatch(m -> m.contains("generated ZDOTDIR")), + "no scrub may run without a configured memberLoginShell — got: " + messages); + } + + /** + * fleetd #213 defect 2, acceptance criterion 3: with {@code memberHerdrSocket} configured, a + * zsh {@code memberLoginShell}, and {@code worktreeRoot}/{@code worktreeGroup} both configured, + * the generated scrub directory must live under {@code worktreeRoot} — NEVER under {@code + * java.io.tmpdir}, which is fleetd's own 0700 temp dir and unreadable by the member's different + * OS user. {@code worktreeGroup} is set to the CURRENT process's own primary group so {@code + * EnvAllowListScrub}'s group-sharing step resolves on whatever host runs this test, rather than + * hardcoding a group name that may not exist here. + */ + @Test + void memberHerdrSocketWithZshMemberLoginShellPutsTheScrubOutsideJavaIoTmpdir(@TempDir Path worktreeRoot) + throws IOException { + String group = currentUserGroup(); + FakeHerdr herdr = new FakeHerdr(); + WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/bash-should-be-ignored", null, + () -> configWithMemberHerdrSocketRootAndGroup("/tmp/other-user-herdr.sock", "/bin/zsh", + worktreeRoot.toString(), group)); + + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + + String zdotdir = launcher.env.get("ZDOTDIR"); + assertNotNull(zdotdir, "a zsh memberLoginShell with worktreeRoot+worktreeGroup configured " + + "must still generate a ZDOTDIR"); + Path dir = Path.of(zdotdir); + // The immediate PARENT is asserted (not merely startsWith(java.io.tmpdir)), because a + // JUnit @TempDir is itself carved out of the JVM's java.io.tmpdir — startsWith alone would + // pass by coincidence of the test fixture, not because the launcher used worktreeRoot. + assertEquals(worktreeRoot.toAbsolutePath().normalize(), dir.getParent(), + "the generated scrub directory's parent must be the configured worktreeRoot, not " + + "System.getProperty(\"java.io.tmpdir\") — got parent " + dir.getParent()); + } + + /** + * fleetd #213, acceptance criterion 4: with {@code memberHerdrSocket} absent (today's only + * mode), behaviour must be byte-identical to before this fix — fleetd's own {@code $SHELL} + * still decides the gate (proven here, not merely assumed, by flagging the lookup), and the + * scrub still lands under {@code java.io.tmpdir}. + */ + @Test + void memberHerdrSocketAbsentStillConsultsFleetdsOwnShellAndBehavesAsBefore() { + FakeHerdr herdr = new FakeHerdr(); + AtomicBoolean shellQueried = new AtomicBoolean(false); + Function env = name -> { + if ("SHELL".equals(name)) { + shellQueried.set(true); + return "/bin/zsh"; + } + return null; + }; + WiringLauncher launcher = new WiringLauncher(herdr, allowList(), env, null); // memberHerdrSocket absent + + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + + assertTrue(shellQueried.get(), "with memberHerdrSocket absent, fleetd's own $SHELL must " + + "still decide the zsh gate, unchanged from before this fix"); + String zdotdir = launcher.env.get("ZDOTDIR"); + assertNotNull(zdotdir, "SHELL=/bin/zsh with memberHerdrSocket absent must still generate a " + + "ZDOTDIR, as before this fix"); + Path dir = Path.of(zdotdir); + assertTrue(dir.startsWith(Path.of(System.getProperty("java.io.tmpdir"))), + "with memberHerdrSocket absent the scrub directory must still be generated under " + + "java.io.tmpdir, unchanged from before this fix: " + dir); + } + + /** The current process's own primary group — resolvable on whatever host runs this test. */ + private static String currentUserGroup() throws IOException { + PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class); + assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership"); + return view.readAttributes().group().getName(); + } + /** Spawn once through the real launcher path, capturing every INFO+ line this class logs. */ private static List spawnAndCaptureLogs(HerdrPeerLauncher launcher) { Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class); @@ -446,6 +595,19 @@ class HerdrPeerLauncherAllowListWiringTest { 0, () -> 0L, () -> { }, null, creds, hostEnvNames, config); } + /** + * Full control over the {@code env} lookup, bypassing the {@code shell}/{@code + * extraEnvValues} convenience above entirely — fleetd #213's "fleetd's own $SHELL must + * never be consulted" tests need to OBSERVE whether {@code SHELL} was ever looked up, which + * a plain value substitution cannot do. + */ + WiringLauncher(FakeHerdr herdr, Supplier creds, + Function env, Supplier config) { + super("test", new AgentControl(herdr), new WorkspaceControl(herdr), + Map.of("test", profile()), "test", + env, 0, () -> 0L, () -> { }, null, creds, null, config); + } + @Override protected Launch buildLaunch(FleetConfig.Profile cfg, LaunchSpec spec) { Map launchEnv = baseEnv(cfg); @@ -477,6 +639,72 @@ class HerdrPeerLauncherAllowListWiringTest { null, null, null, null, null, null, null, null, null, null, null).withDefaults(); } + /** + * fleetd #213: as {@link #configWithMemberHerdrSocket(String)}, plus the {@code + * memberLoginShell:} the member's OS user actually runs — the config key {@link + * HerdrPeerLauncher#applyEnvironmentAllowListPolicy} must consult instead of fleetd's own + * {@code $SHELL} once {@code memberHerdrSocket} is configured. + */ + private static FleetConfig configWithMemberHerdrSocket(String memberHerdrSocket, String memberLoginShell) { + return new FleetConfig( + null, // bind + null, // herdrSocket + memberHerdrSocket, // memberHerdrSocket + Map.of(), // profiles + null, // guard + null, // worktreeRoot + null, // lifecycle + null, // spawnReadyTimeoutMs + null, // spawnReadyPollMs + null, // broker + null, // primary + null, // fleet + null, // leadHeartbeat + null, // health + null, // placement + null, // auth + null, // configReload + null, // quarantineCooldownSeconds + null, // memberCredentials + null, // coordinator + null, // worktreeGroup + memberLoginShell // memberLoginShell + ).withDefaults(); + } + + /** + * 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}. + */ + private static FleetConfig configWithMemberHerdrSocketRootAndGroup(String memberHerdrSocket, + String memberLoginShell, String worktreeRoot, String worktreeGroup) { + return new FleetConfig( + null, // bind + null, // herdrSocket + memberHerdrSocket, // memberHerdrSocket + Map.of(), // profiles + null, // guard + worktreeRoot, // worktreeRoot + null, // lifecycle + null, // spawnReadyTimeoutMs + null, // spawnReadyPollMs + null, // broker + null, // primary + null, // fleet + null, // leadHeartbeat + null, // health + null, // placement + null, // auth + null, // configReload + null, // quarantineCooldownSeconds + null, // memberCredentials + null, // coordinator + worktreeGroup, // worktreeGroup + memberLoginShell // memberLoginShell + ).withDefaults(); + } + /** The generated directory is a temp directory; make sure the test does not leave a pile. */ @Test void theGeneratedDirectoryIsRemovedWhenThePaneIsStopped() {