diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index c63aa07..4988c95 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -177,14 +177,14 @@ public final class Fleetd { claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), () -> config.get().fleet(), - () -> config.get().memberCredentials())); + () -> config.get().memberCredentials(), null, config::get)); } if (!opencodeProfiles.isEmpty()) { adapters.add(new OpenCodeLauncher(router.memberAgents(), router.memberSpaces(), opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), () -> config.get().fleet(), - () -> config.get().memberCredentials())); + () -> config.get().memberCredentials(), config::get)); } AtomicReference> liveCountRef = new AtomicReference<>(_ -> 0); // CB-578 stage B: one quarantine tracker for the whole daemon, shared between the launcher diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java index 3079c0e..0e55579 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java @@ -94,13 +94,26 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, Map profiles, String defaultProfile, Function env, - long spawnReadyTimeoutMs, long spawnReadyPollMs, - Supplier fleet, - Supplier memberCredentials) { + long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials) { + this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, spawnReadyPollMs, + fleet, memberCredentials, null, null); + } + + /** Production constructor, plus the live config for URI environment exclusions. */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials, + Supplier> hostEnvNames, + Supplier config) { this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), - fleet, memberCredentials); + fleet, memberCredentials, hostEnvNames, config); } /** @@ -153,10 +166,22 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, - Supplier fleet, - Supplier memberCredentials) { + Supplier fleet, + Supplier memberCredentials) { + this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, + fleet, memberCredentials, null, null); + } + + /** Full testability constructor, plus the live config for URI environment exclusions. */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, Supplier fleet, + Supplier memberCredentials, + Supplier> hostEnvNames, + Supplier config) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, - spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials); + spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames, config); this.guard = guard; } @@ -174,7 +199,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { Supplier memberCredentials, Supplier> hostEnvNames) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, - spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames); + spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames, null); this.guard = guard; } 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 113fb22..25fd025 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -170,6 +170,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { /** Guards {@link #warnNonZsh} to one WARN per launcher instance, not one per spawn. */ private final AtomicBoolean nonZshShellWarned = new AtomicBoolean(); + /** Live config provides URI environment names that must never enter member panes. */ + private final Supplier config; /** * @param namePrefix label prefix for this peer kind (drives naming and reap) @@ -238,8 +240,19 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, Supplier fleet, + Supplier memberCredentials, + Supplier> hostEnvNames) { + this(namePrefix, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, + sleeper, fleet, memberCredentials, hostEnvNames, null); + } + + /** As above, plus the live full config for secret-bearing URI environment names. */ + protected HerdrPeerLauncher(String namePrefix, AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, Supplier fleet, Supplier memberCredentials, - Supplier> hostEnvNames) { + Supplier> hostEnvNames, Supplier config) { this.fleet = fleet; this.namePrefix = namePrefix; this.agents = agents; @@ -252,6 +265,7 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { this.sleeper = sleeper; this.memberCredentials = memberCredentials; this.hostEnvNames = hostEnvNames != null ? hostEnvNames : () -> System.getenv().keySet(); + this.config = config; } // --- adapter seams ------------------------------------------------------------------------- @@ -1028,9 +1042,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { } /** Put {@link #BLOCKED_CREDENTIAL_SENTINEL} over every blocked name in the pane-creation env map. */ - private static void overlayBlockedCredentials(Map workerEnv, - FleetConfig.MemberCredentials creds) { - for (String name : creds.blockedSet()) { + private void overlayBlockedCredentials(Map workerEnv, + FleetConfig.MemberCredentials creds) { + Set blocked = new java.util.TreeSet<>(creds.blockedSet()); + blocked.addAll(brokerUriEnvNames()); + for (String name : blocked) { workerEnv.put(name, BLOCKED_CREDENTIAL_SENTINEL); } } @@ -1097,15 +1113,21 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * ssh-agent handle when explicitly allowed, and the exact keys of THIS launch's own env map. */ private Set derivedAllowedNames(FleetConfig.MemberCredentials creds, Launch launch) { + Set brokerUriEnvNames = brokerUriEnvNames(); Set allowed = new java.util.TreeSet<>( - MemberEnvAllowList.derive(profiles.values(), creds.allowSet())); + MemberEnvAllowList.derive(profiles.values(), creds.allowSet(), brokerUriEnvNames)); if (creds.sshAuthSockAllowed()) { allowed.add(SSH_AUTH_SOCK); } // blocked by default: absent from the set ⇒ blanked by the scrub like any other name allowed.addAll(launch.env().keySet()); + allowed.removeAll(brokerUriEnvNames); return allowed; } + private Set brokerUriEnvNames() { + return MemberEnvAllowList.brokerUriEnvNames(config == null ? null : config.get()); + } + /** * CB-633 follow-up: one INFO line per allow-list spawn WHOSE SCRUB ACTUALLY RUNS, so an operator * can read a single log line and know the scrub ran and how much of the visible environment it diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java b/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java index 8ed0872..07d927b 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java @@ -40,8 +40,9 @@ import java.util.TreeSet; * operator's own explicit list. Before this, {@code policy: allow-list} silently ignored every name * an operator wrote under {@code allow:} unless a profile happened to carry it too, which meant * turning the policy on could blank credentials working members already depended on. {@code - * SSH_AUTH_SOCK} is the one exception: even when the operator lists it under {@code allow:}, it is - * excluded here and added back ONLY by the caller when {@code sshAuthSock: allow} is explicitly set + * SSH_AUTH_SOCK} and configured broker URI environment names are exceptions: even when the operator + * lists them under {@code allow:}, they are excluded here. {@code SSH_AUTH_SOCK} is added back ONLY + * by the caller when {@code sshAuthSock: allow} is explicitly set * (see {@link #SSH_AUTH_SOCK}'s javadoc) — it is a live handle to the operator's own ssh-agent, not * a value, so treating it like any other allow-listed name would hand a member every key the * operator's agent holds the moment they typed the name under {@code allow:} for an unrelated @@ -106,6 +107,16 @@ public final class MemberEnvAllowList { * run-to-run. */ public static Set derive(Collection profiles, Set configuredAllow) { + return derive(profiles, configuredAllow, Set.of()); + } + + /** + * As {@link #derive(Collection, Set)}, while excluding names that fleetd knows carry credentials. + * A configured broker URI contains its AMQP password inline, so it must never reach a member, + * even when an operator put its variable name in {@code memberCredentials.allow:}. + */ + public static Set derive(Collection profiles, Set configuredAllow, + Set excludedNames) { Set derived = new TreeSet<>(INFRASTRUCTURE_PASSTHROUGH); if (profiles != null) { for (FleetConfig.Profile p : profiles) { @@ -124,9 +135,37 @@ public final class MemberEnvAllowList { } } } + if (excludedNames != null) { + derived.removeAll(excludedNames); + } return Set.copyOf(derived); } + /** + * The host environment names whose values are AMQP URIs with inline passwords. Both broker + * connections belong to fleetd, never to a member pane. Blank and absent configuration changes + * nothing. + * + *

How strong this exclusion is depends on the policy, and the difference matters. + * Under {@code policy: allow-list} it is enforced by the generated ZDOTDIR scrub, which runs + * AFTER the pane's shell has sourced the operator's chain — so a login shell that re-exports the + * name is still blanked. Under the deny-list policy there is no scrub: the name is only removed + * from the pre-shell env map, and a login shell that sources the operator's secret store + * re-exports it. That is the long-standing weakness of deny-list (a sourced file can undo it), + * not something this exclusion introduces, but it means deny-list deployments do NOT get this + * guarantee. The same caveat applies to the non-zsh path, which has no scrub at all — see + * {@code HerdrPeerLauncher#applyEnvironmentAllowListPolicy}. + */ + public static Set brokerUriEnvNames(FleetConfig config) { + if (config == null) { + return Set.of(); + } + Set names = new TreeSet<>(); + addUriEnvIfPresent(names, config.broker()); + addUriEnvIfPresent(names, config.coordinator()); + return Set.copyOf(names); + } + /** * Whether {@code name} survives the scrub when {@code allowedNames} is the derived set: an exact * match, or an infrastructure-prefixed name ({@code LC_*}). Prefix rules live ONLY here and in @@ -146,4 +185,16 @@ public final class MemberEnvAllowList { into.add(name); } } + + private static void addUriEnvIfPresent(Set into, FleetConfig.Broker broker) { + if (broker != null && broker.hasUriEnv()) { + into.add(broker.uriEnv()); + } + } + + private static void addUriEnvIfPresent(Set into, FleetConfig.Coordinator coordinator) { + if (coordinator != null && coordinator.hasUriEnv()) { + into.add(coordinator.uriEnv()); + } + } } diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java index 0401d07..7f75538 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java @@ -116,12 +116,23 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, Map profiles, String defaultProfile, Function env, - long spawnReadyTimeoutMs, long spawnReadyPollMs, - Supplier fleet, - Supplier memberCredentials) { + long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, spawnReadyPollMs, + fleet, memberCredentials, null); + } + + /** Production constructor, plus the live config for URI environment exclusions. */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials, + Supplier config) { this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), - defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials); + defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials, config); } /** @@ -182,8 +193,20 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { Path configRoot, Path discoveryRoot, Supplier fleet, Supplier memberCredentials) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper, + configRoot, discoveryRoot, fleet, memberCredentials, null); + } + + /** Full testability constructor, plus the live config for URI environment exclusions. */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, Path configRoot, Path discoveryRoot, + Supplier fleet, + Supplier memberCredentials, + Supplier config) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, - spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials); + spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, null, config); this.configRoot = configRoot; this.discovery = new OpenCodeSessionDiscovery(discoveryRoot); } 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 2e926b2..e68d729 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -161,6 +161,34 @@ class HerdrPeerLauncherAllowListWiringTest { + "it under allow: — sshAuthSock is unset here, so it defaults to block"); } + @Test + void brokerUriEnvStaysBlockedWhenListedInMemberCredentialsAllow() { + FakeHerdr herdr = new FakeHerdr(); + WiringLauncher launcher = new WiringLauncher(herdr, + allowListWithAllow(List.of("BROKER_CONNECTION_URI")), "/bin/zsh", null, + () -> config("BROKER_CONNECTION_URI")); + + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + + Path dir = Path.of(launcher.env.get("ZDOTDIR")); + assertFalse(readAll(dir.resolve(EnvAllowListScrub.SCRUB_FILE)).contains("'BROKER_CONNECTION_URI'"), + "broker.uriEnv must not reach a member even when listed in memberCredentials.allow:"); + } + + @Test + void brokerUriEnvIsDeniedUnderTheDenyListPolicyEvenWhenAllowed() { + FakeHerdr herdr = new FakeHerdr(); + WiringLauncher launcher = new WiringLauncher(herdr, + () -> new FleetConfig.MemberCredentials(null, List.of("BROKER_CONNECTION_URI"), List.of(), null), + "/bin/bash", null, + () -> config("BROKER_CONNECTION_URI")); + + launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV)); + + assertEquals("blocked-by-fleetd-cb596-see-gitea-issue-82", launcher.env.get("BROKER_CONNECTION_URI"), + "the deny-list overlay must deny broker.uriEnv even when allow: names it"); + } + /** * CB-633 follow-up criterion 3: on every allow-list spawn the daemon logs one INFO line, shaped * "member credentials: allowed N of M", with real counts — not constants. Real path: the count @@ -260,15 +288,24 @@ class HerdrPeerLauncherAllowListWiringTest { /** Plus an injectable {@code hostEnvNames} source, for the "allowed N of M" log line test. */ WiringLauncher(FakeHerdr herdr, Supplier creds, String shell, - Supplier> hostEnvNames) { + Supplier> hostEnvNames) { + this(herdr, creds, shell, hostEnvNames, null); + } + + WiringLauncher(FakeHerdr herdr, Supplier creds, String shell, + Supplier> hostEnvNames, Supplier config) { super("test", new AgentControl(herdr), new WorkspaceControl(herdr), Map.of("test", profile()), "test", name -> "SHELL".equals(name) ? shell : null, - 0, () -> 0L, () -> { }, null, creds, hostEnvNames); + 0, () -> 0L, () -> { }, null, creds, hostEnvNames, config); } @Override protected Launch buildLaunch(FleetConfig.Profile cfg, LaunchSpec spec) { + Map launchEnv = baseEnv(cfg); + launchEnv.putAll(env); + env.clear(); + env.putAll(launchEnv); return new Launch(env, List.of("test")); } @@ -278,6 +315,12 @@ class HerdrPeerLauncherAllowListWiringTest { } } + private static FleetConfig config(String brokerUriEnv) { + return new FleetConfig(null, null, null, Map.of(), null, null, null, null, null, + new FleetConfig.Broker(null, brokerUriEnv, 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() { diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java index 3cbb1b3..bc0f9a7 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java @@ -121,6 +121,34 @@ class MemberEnvAllowListTest { assertTrue(derived.contains("OTHER_NAME"), "other allow: names are unaffected"); } + @Test + void configuredBrokerAndCoordinatorUriEnvNamesAreExcludedEvenWhenAllowed() { + FleetConfig config = config("BROKER_CONNECTION_URI", "COORDINATOR_CONNECTION_URI"); + Set excluded = MemberEnvAllowList.brokerUriEnvNames(config); + + Set derived = MemberEnvAllowList.derive(List.of(), + Set.of("BROKER_CONNECTION_URI", "COORDINATOR_CONNECTION_URI", "OTHER_NAME"), excluded); + + assertFalse(derived.contains("BROKER_CONNECTION_URI"), + "broker.uriEnv is secret-bearing and must not ride in on allow:"); + assertFalse(derived.contains("COORDINATOR_CONNECTION_URI"), + "coordinator.uriEnv has the same inline-password shape"); + assertTrue(derived.contains("OTHER_NAME"), "unrelated allow: entries are unaffected"); + } + + @Test + void absentOrBlankBrokerUriEnvAddsNoExclusions() { + assertTrue(MemberEnvAllowList.brokerUriEnvNames(config(null, null)).isEmpty()); + assertTrue(MemberEnvAllowList.brokerUriEnvNames(config(" ", "")).isEmpty()); + } + + private static FleetConfig config(String brokerUriEnv, String coordinatorUriEnv) { + return new FleetConfig(null, null, null, Map.of(), null, null, null, null, null, + new FleetConfig.Broker(null, brokerUriEnv, null), null, null, null, null, null, + null, null, null, null, + new FleetConfig.Coordinator(null, coordinatorUriEnv, null, null)).withDefaults(); + } + /** {@code LC_*} categories are infrastructure by prefix; everything else needs an exact match. */ @Test void keepsMatchesExactlyPlusTheLocalePrefixRule() {