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 25fd025..97b16d7 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -112,6 +112,12 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * daemon's own process is started the same way (a login shell sourcing the same secret store — * see CB-592's investigation of {@code secrets.sh}), so on a single-host deployment its env * mirrors what the pane's login shell is about to export. + * + *

fleetd #185 stage 2: that mirroring assumption holds only while the member pane runs under + * the SAME OS user as the daemon. When {@code memberHerdrSocket:} is configured, member panes + * run on a second herdr owned by a different user — different {@code $HOME}, different {@code + * secrets.sh}, different environment entirely — so this field's data no longer describes what a + * member pane inherits. See {@link #logCredentialGap} for how that mode is handled. */ private final Supplier> hostEnvNames; @@ -1230,6 +1236,67 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { */ private final AtomicBoolean allowListGapLogged = new AtomicBoolean(); + /** + * fleetd #185 stage 2: guards {@link #warnUnknownMemberEnvironment} to one WARN per launcher + * instance, not one per spawn — the same one-per-instance shape as {@link #unprotectedGapLogged} + * and {@link #allowListGapLogged}, kept as its own flag for the same reason those two are split: + * this mode is orthogonal to which of the other two branches would otherwise have fired. + */ + private final AtomicBoolean unknownMemberEnvironmentWarned = new AtomicBoolean(); + + /** + * fleetd #185 stage 2: whether {@code memberHerdrSocket:} is configured, i.e. member panes run + * on a second herdr owned by a different OS user than the daemon's own process. Re-read from the + * live config on every call (same hot-reload shape as {@link #memberCredentials}), never cached, + * so a config reload takes effect on the next spawn without a restart. + * + *

{@link #config} is {@code null} on any call site that never threaded the full config + * through (every production {@code HerdrPeerLauncher} does; a handful of older tests do not) — + * treated the same as "not configured", which is the correct, permissive default: it is exactly + * today's single-daemon behaviour. + */ + private boolean memberHerdrSocketConfigured() { + if (config == null) { + return false; + } + FleetConfig cfg = config.get(); + return cfg != null && cfg.memberHerdrSocket() != null && !cfg.memberHerdrSocket().isBlank(); + } + + /** + * fleetd #185 stage 2: the single replacement WARN for {@link #logCredentialGap}'s usual + * conclusions when {@code memberHerdrSocket:} is configured. {@link #hostEnvNames} (and + * everything derived from it — {@code known}/{@code allow} coverage, the allow-list scrub's + * derived set) describes the DAEMON's own environment; under this config key member panes run as + * a different OS user with a different environment entirely, so neither "every member pane + * inherits them UNBLOCKED" nor "the scrub blanks them" is evidence-backed here — both would be + * reporting on the wrong process. Logged once, names the config key, and states the honest + * conclusion: the gap for member panes is UNKNOWN, not clean, so {@code memberCredentials} cannot + * be verified from this daemon. The one count it does report is scoped explicitly to fleetd's own + * environment, never presented as if it said anything about the member's — see {@link + * #logCredentialGap}'s javadoc for why this branch exists. + */ + private void warnUnknownMemberEnvironment(FleetConfig.MemberCredentials creds) { + if (!unknownMemberEnvironmentWarned.compareAndSet(false, true)) { + return; + } + Set covered = new HashSet<>(creds.known()); + covered.addAll(creds.allow()); + Set hostNames = hostEnvNames.get(); + long gapInFleetdsOwnEnv = hostNames.stream() + .filter(name -> CREDENTIAL_SHAPED_NAME.matcher(name).matches()) + .filter(name -> !covered.contains(name)) + .count(); + log.warn("memberCredentials gap: memberHerdrSocket is configured, so member panes run under " + + "a different OS user than fleetd's own process, with a different environment " + + "entirely — fleetd has no channel to read that user's environment. {} of the " + + "{} names in fleetd's OWN environment are credential-shaped and not on " + + "known:/allow:, but that count describes fleetd's process, not the member " + + "herdr's. The credential gap for member panes is UNKNOWN, not clean, and " + + "memberCredentials cannot be verified from here.", + gapInFleetdsOwnEnv, hostNames.size()); + } + /** * CB-596 criterion 4: a credential-shaped host env var name on neither {@code known} nor * {@code allow} is not silently allowed — it is reported. {@link #hostEnvNames} enumerates the @@ -1258,8 +1325,22 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * (same severity, and same guard, as the deny-by-default case — a name genuinely reaching a * member unprotected is equally serious whichever path put it there), and the names it says are * blanked keep the INFO. + * + *

fleetd #185 stage 2: everything above assumes the member pane runs under the same OS user + * as the daemon, so {@link #hostEnvNames} mirrors what the pane inherits — see that field's + * javadoc. When {@code memberHerdrSocket:} is configured that assumption is false: the member + * pane runs on a second herdr owned by a different user, and neither conclusion below + * ("inherits them UNBLOCKED" / "the scrub blanks them") is backed by evidence about that user's + * environment. So this method checks that first and, when configured, reports the honest + * "unknown, not clean" conclusion instead — see {@link #warnUnknownMemberEnvironment}. When + * {@code memberHerdrSocket:} is absent (the default, and the only mode this host runs) this + * branch is never taken and every line below is unchanged. */ private void logCredentialGap(FleetConfig.MemberCredentials creds, Set effectiveAllowed) { + if (memberHerdrSocketConfigured()) { + warnUnknownMemberEnvironment(creds); + return; + } Set covered = new HashSet<>(creds.known()); covered.addAll(creds.allow()); List gap = hostEnvNames.get().stream() 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 e68d729..2a66b7e 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -257,6 +257,137 @@ class HerdrPeerLauncherAllowListWiringTest { + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList()); } + /** + * fleetd #185 stage 2 pin: with {@code memberHerdrSocket:} absent (today's only mode, and the + * default — this host runs no other), the gap detector's WARN/INFO conclusions read exactly as + * they did before this fix. Real path: {@code FLEETD_WORKER_TOKEN} (the test profile's own + * {@code tokenEnv}) is a name the derived allow-list keeps, so it gets the "UNBLOCKED" WARN; + * {@code SOME_UNKNOWN_SECRET_TOKEN} is not derived from anywhere, so it gets the "scrub blanks + * them" INFO. This is the exact shape #185 stage 2 must not touch on this path. + */ + @Test + void gapConclusionsAreByteIdenticalWhenMemberHerdrSocketIsAbsent() { + FakeHerdr herdr = new FakeHerdr(); + Set hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN"); + WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames, + () -> config(null)); + + List messages = spawnAndCaptureLogs(launcher); + + assertTrue(messages.contains("memberCredentials gap: 1 credential-shaped env var name(s) are on " + + "neither known: nor allow: — the derived allow-list keeps them anyway (a " + + "profile's gitTokenEnv/gitHostEnv/tokenEnv/env: names one, or this spawn " + + "injects it), so every member pane inherits them UNBLOCKED — [FLEETD_WORKER_TOKEN]. " + + "Add each to memberCredentials.known (or .allow if a member legitimately needs " + + "it), or remove it from whatever profile setting derives it in."), + "expected the pre-existing UNBLOCKED WARN unchanged, got: " + messages); + assertTrue(messages.contains("memberCredentials gap: 1 credential-shaped env var name(s) are on " + + "neither known: nor allow: — [SOME_UNKNOWN_SECRET_TOKEN]. The allow-list scrub " + + "blanks them anyway (they are not on the derived allow-list), so no member pane " + + "keeps them; add each to memberCredentials.known or .allow to make that explicit."), + "expected the pre-existing 'scrub blanks them' INFO unchanged, got: " + messages); + } + + /** + * fleetd #185 stage 2: with {@code memberHerdrSocket:} configured, member panes run under a + * 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. + */ + @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")); + + List messages = spawnAndCaptureLogs(launcher); + + assertTrue(messages.stream().anyMatch(m -> m.contains("memberHerdrSocket") + && m.contains("UNKNOWN") && m.contains("cannot be verified")), + "expected the unknown-member-environment WARN naming memberHerdrSocket, got: " + messages); + assertFalse(messages.stream().anyMatch(m -> m.contains("UNBLOCKED")), + "the 'inherits them UNBLOCKED' conclusion must not print once the evidence is about " + + "the wrong (daemon's own) environment — got: " + messages); + assertFalse(messages.stream().anyMatch(m -> m.contains("scrub blanks them")), + "the 'scrub blanks them' conclusion must not print once the evidence is about the " + + "wrong (daemon's own) environment — got: " + messages); + } + + /** + * fleetd #185 stage 2: the unknown-environment WARN is a standing fact about this launcher's + * configuration, not per-spawn news — it must fire once per launcher instance, the same shape as + * every other one-time WARN in this class (e.g. {@code warnNonZsh}). + */ + @Test + void theUnknownEnvironmentWarnFiresOnceNotOncePerSpawn() { + 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")); + + Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class); + Level original = logger.getLevel(); + logger.setLevel(Level.WARN); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + try { + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + } finally { + logger.detachAppender(appender); + logger.setLevel(original); + } + + long count = appender.list.stream() + .filter(e -> e.getFormattedMessage().contains("cannot be verified from here")) + .count(); + assertEquals(1, count, "the unknown-member-environment WARN must fire once per launcher " + + "instance, not once per spawn — got " + count + " occurrence(s) among: " + + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList()); + } + + /** + * Hard constraint: the gap detector must never log an env var VALUE, only its NAME. {@code + * SOME_UNKNOWN_SECRET_TOKEN} resolves to a distinctive canary value through the same {@code env} + * lookup the launcher uses elsewhere (SHELL, PATH, token resolution) — proving the value IS + * resolvable does not mean the detector reads it, since {@link HerdrPeerLauncher#hostEnvNames} + * (names only) is its data source, never {@code env.apply(name)} for those names. + */ + @Test + void theGapDetectorNeverLogsAnEnvVarValueOnlyItsName() { + FakeHerdr herdr = new FakeHerdr(); + String canary = "sekrit-value-CANARY-9f3a1b7c"; + Set hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN"); + WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames, + () -> config(null), Map.of("SOME_UNKNOWN_SECRET_TOKEN", canary)); + + List messages = spawnAndCaptureLogs(launcher); + + assertTrue(messages.stream().anyMatch(m -> m.contains("SOME_UNKNOWN_SECRET_TOKEN")), + "expected the credential-shaped NAME to appear in the log, got: " + messages); + assertFalse(messages.stream().anyMatch(m -> m.contains(canary)), + "the log must never contain an env var VALUE, only its NAME — got: " + messages); + } + + /** 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); + Level original = logger.getLevel(); + logger.setLevel(Level.INFO); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + try { + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + } finally { + logger.detachAppender(appender); + logger.setLevel(original); + } + return appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList(); + } + private static String readAll(Path p) { try { return Files.readString(p); @@ -294,9 +425,24 @@ class HerdrPeerLauncherAllowListWiringTest { WiringLauncher(FakeHerdr herdr, Supplier creds, String shell, Supplier> hostEnvNames, Supplier config) { + this(herdr, creds, shell, hostEnvNames, config, Map.of()); + } + + /** + * Plus a host-env value map (name → value), resolved through the same {@code env} lookup + * every adapter uses for {@code SHELL}/{@code PATH}/token resolution — fleetd #185 stage 2's + * "the gap detector never logs a value" tests use this to prove a value that IS resolvable + * for a credential-shaped name never reaches the log, since the detector only ever reads + * {@code hostEnvNames} (names), never {@code env.apply(name)} (values), for those names. + */ + WiringLauncher(FakeHerdr herdr, Supplier creds, String shell, + Supplier> hostEnvNames, Supplier config, + Map extraEnvValues) { super("test", new AgentControl(herdr), new WorkspaceControl(herdr), Map.of("test", profile()), "test", - name -> "SHELL".equals(name) ? shell : null, + name -> "SHELL".equals(name) ? shell + : (extraEnvValues != null && extraEnvValues.containsKey(name)) + ? extraEnvValues.get(name) : null, 0, () -> 0L, () -> { }, null, creds, hostEnvNames, config); } @@ -321,6 +467,16 @@ class HerdrPeerLauncherAllowListWiringTest { null, null, null, null, null).withDefaults(); } + /** + * fleetd #185 stage 2: a config with {@code memberHerdrSocket:} set — member panes run on a + * second herdr owned by a different OS user, so {@link HerdrPeerLauncher#hostEnvNames} no + * longer describes what a member pane inherits. + */ + private static FleetConfig configWithMemberHerdrSocket(String memberHerdrSocket) { + return new FleetConfig(null, null, memberHerdrSocket, Map.of(), null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, null).withDefaults(); + } + /** The generated directory is a temp directory; make sure the test does not leave a pile. */ @Test void theGeneratedDirectoryIsRemovedWhenThePaneIsStopped() {