From d432df8e5ccb20236b4b5ef9fa5d54be6899651d Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sun, 23 Aug 2026 07:43:38 +0200 Subject: [PATCH] =?UTF-8?q?CB-633:=20memberCredentials=20policy=3Dallow-li?= =?UTF-8?q?st=20=E2=80=94=20derived=20ZDOTDIR=20env=20scrub?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move member environment control out of the pane-creation env overlay (defeated by any file the login shell sources) into a per-spawn ZDOTDIR directory whose .zlogin runs LAST, after the operator's whole chain, and blanks every exported variable not on an allow-list DERIVED from what the launcher itself injects (profiles' tokenEnv/gitTokenEnv/gitHostEnv/env keys + an infrastructure set) — never hand-typed. - memberCredentials.policy: allow-list (deny-by-default/deny-list stay default and unchanged); known:/allow: become reporting only under it. - memberCredentials.sshAuthSock knob, blocked by default; allowing it is an explicit decision (operator ssh-agent handle). - Non-zsh login shell: loud WARN, protection off, fallback to the old enumerated-name overlay. - Scrub writes an 'allowed N of M' denominator report, read at teardown; credential-shaped blanked names go to WARN (names only, never values). - Equality test against a real login zsh from a clean parent: surviving non-empty exports EQUAL baseline ∩ derived allow-list. --- bridged/fleetd.example.yaml | 42 +++- .../src/main/java/dev/ltms/fleet/Fleetd.java | 8 +- .../dev/ltms/fleet/config/FleetConfig.java | 94 +++++++-- .../ltms/fleet/member/EnvAllowListScrub.java | 193 ++++++++++++++++++ .../ltms/fleet/member/HerdrPeerLauncher.java | 135 +++++++++++- .../ltms/fleet/member/MemberEnvAllowList.java | 108 ++++++++++ .../ltms/fleet/config/FleetConfigTest.java | 60 ++++++ .../fleet/member/EnvAllowListScrubTest.java | 153 ++++++++++++++ .../fleet/member/MemberEnvAllowListTest.java | 105 ++++++++++ 9 files changed, 872 insertions(+), 26 deletions(-) create mode 100644 bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java create mode 100644 bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java create mode 100644 bridged/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java create mode 100644 bridged/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java diff --git a/bridged/fleetd.example.yaml b/bridged/fleetd.example.yaml index 7c6ad88..0d17170 100644 --- a/bridged/fleetd.example.yaml +++ b/bridged/fleetd.example.yaml @@ -505,20 +505,44 @@ guard: # through either: the daemon logs a WARN naming any credential-shaped env var it finds on neither # list (never its value), so a secret added to the store later does not go unnoticed forever. # -# policy → only "deny-by-default" exists today (an operator-authored deny-list was deliberately -# rejected — see above). An unrecognized value refuses to start, naming it. -# allow → credential names a member legitimately needs. Left OUT of the pane's env overlay -# entirely, so the value the pane's own (login) shell exports passes through untouched. -# known → every credential name the operator's store is known to export. Every name here NOT -# also in `allow` is overlaid with a non-secret sentinel value before the pane's login -# shell runs — real protection only for names the login shell does not itself re-export -# (see the ROUND-2 CORRECTION note above for the ones it does). +# policy → "deny-by-default" (the default; also accepted spelled "deny-list") overlays each +# known-but-not-allowed name BEFORE the pane's login shell runs — real protection only +# where that shell does not re-export the name (see ROUND-2 CORRECTION above). An +# unrecognized value refuses to start, naming it. +# policy → "allow-list" (CB-633) moves the control to a per-spawn ZDOTDIR directory the daemon +# generates and passes through tab.create's env map. The pane's zsh startup order is +# .zshenv → .zprofile → .zshrc → .zlogin, and the operator's whole chain runs inside the +# first three — so the generated .zlogin, which sources ~/.zlogin first and THEN blanks +# every exported variable not on the derived allow-list, runs after everything the +# operator sourced. No sourced file can undo it. The allow-list is DERIVED, never typed: +# every profile's tokenEnv/gitTokenEnv/gitHostEnv values and env-map keys, plus an +# infrastructure set (PATH HOME SHELL TERM LANG LC_* TMPDIR USER LOGNAME PWD SHLVL EDITOR +# PAGER JAVA_HOME XDG_* ZDOTDIR), plus whatever keys this spawn's own env overlay carries. +# Adding a profile can therefore only widen the list, never break another spawn's scrub. +# Under this policy `known`/`allow` below become REPORTING ONLY — they feed the gap WARN, +# they are no longer a control. If the member's login shell is NOT zsh, the daemon logs a +# loud WARN saying protection is off and falls back to deny-by-default's overlay. +# allow → credential names a member legitimately needs. Under deny-by-default, left OUT of the +# pane's env overlay entirely, so the value the pane's own (login) shell exports passes +# through untouched. Under allow-list: reporting only. +# known → every credential name the operator's store is known to export. Under deny-by-default, +# every name here NOT also in `allow` is overlaid with a non-secret sentinel value before +# the pane's login shell runs — real protection only for names that shell does not itself +# re-export (see the ROUND-2 CORRECTION note above). Under allow-list: reporting only. +# sshAuthSock → whether SSH_AUTH_SOCK may pass through under allow-list ("allow") or must be +# blanked like any other non-derived name ("block", the default). This is a decision you +# have to make explicitly: SSH_AUTH_SOCK is a handle to YOUR ssh-agent, and a member +# holding it can sign with your keys — it sits in no secret file and looks like no +# credential, which is why it slipped past three earlier tickets (gitea #110). Blocking +# it breaks git over SSH inside members (push/fetch authenticate as you); use HTTPS +# remotes or scoped deploy keys instead of allowing it lightly. # # HOT-RELOADABLE the same way `fleet:` is (CB-559): read fresh on every spawn, so editing this list # and reloading config (or restarting) changes what the NEXT spawn inherits; already-running members # are unaffected either way. # memberCredentials: -# policy: deny-by-default +# policy: deny-by-default # or "deny-list", or "allow-list" (CB-633) — see above +# sshAuthSock: block # allow-list only; see the sshAuthSock note above # allow: # - AI_GATEWAY_TOKEN # named in a profile's tokenEnv (local/gx) — a member reaching the # # gateway is by design, not a leak diff --git a/bridged/src/main/java/dev/ltms/fleet/Fleetd.java b/bridged/src/main/java/dev/ltms/fleet/Fleetd.java index 5838b97..72c118c 100644 --- a/bridged/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/bridged/src/main/java/dev/ltms/fleet/Fleetd.java @@ -652,8 +652,12 @@ public final class Fleetd { static void reportMemberCredentialsGap(FleetConfig cfg) { FleetConfig.MemberCredentials creds = cfg.memberCredentials(); if (creds != null && !creds.known().isEmpty()) { - log.info("memberCredentials: {} known name(s), {} allowed — blocking {} on every spawn", - creds.known().size(), creds.allow().size(), creds.blockedSet().size()); + log.info("memberCredentials: policy={}, {} known name(s), {} allowed — blocking {} on " + + "every spawn{}", + creds.policy(), creds.known().size(), creds.allow().size(), creds.blockedSet().size(), + creds.isAllowList() + ? " (allow-list: known/allow are reporting only — the control is the derived ZDOTDIR scrub)" + : ""); return; } log.warn("memberCredentials: absent or empty — the daemon will start anyway, and every " diff --git a/bridged/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/bridged/src/main/java/dev/ltms/fleet/config/FleetConfig.java index 66641c6..455a65d 100644 --- a/bridged/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/bridged/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -957,27 +957,84 @@ public record FleetConfig( * UNLESS it is also in {@link #allow}. A name that shows up in neither list is not silently * allowed — see {@code HerdrPeerLauncher}'s gap detector, which logs it. * - * @param policy how the block is computed. Only {@link #POLICY_DENY_BY_DEFAULT} is understood - * today; {@code null}/blank defaults to it. An operator's own deny-list is - * deliberately not supported — see above. + *

CB-633: allow-list. Deny-by-default's overlay is applied BEFORE the pane's login + * shell runs, so any file that chain sources can re-export over it — and did. The allow-list + * policy moves the control to a generated ZDOTDIR whose {@code .zlogin} runs LAST, after the + * whole operator chain, and blanks every exported variable not on an allow-list DERIVED from + * what the launcher itself injects (profiles' tokenEnv/gitTokenEnv/gitHostEnv/env keys plus an + * infrastructure set) — never hand-typed, so adding a profile cannot break a spawn. Under this + * policy {@link #allow} and {@link #known} stop being a control and become reporting only. + * + * @param policy how the block is computed. {@link #POLICY_DENY_BY_DEFAULT} (the default; also + * accepted as {@link #POLICY_DENY_LIST}) shadows each {@code known}-but-not-allowed + * name in the pane-creation env overlay — which a login shell that re-exports the + * name defeats (see CB-596's round-2 correction). {@link #POLICY_ALLOW_LIST} + * (CB-633) replaces the overlay with a per-spawn ZDOTDIR scrub that runs AFTER the + * pane's login shell has finished sourcing everything, blanking every variable not + * on the DERIVED allow-list (derived from what the launcher itself injects — never + * hand-typed). Under {@code allow-list}, {@link #allow} and {@link #known} are + * REPORTING ONLY: they feed the gap WARN, they are no longer a control. * @param allow credential names a member legitimately needs (e.g. the gateway token it reaches - * the LLM through, the repo-scoped forge token it opens its own PR with). Every - * name here is left unmentioned in the pane's env overlay, so the value the pane's - * own (login) shell exports passes through untouched. - * @param known every credential name the operator's store is known to export. Every name here - * that is NOT also in {@link #allow} is overlaid with a non-secret sentinel value, - * shadowing whatever the pane's login shell would otherwise export for it. + * the LLM through, the repo-scoped forge token it opens its own PR with). Under + * deny-list/deny-by-default every name here is left unmentioned in the pane's env + * overlay; under allow-list this list is reporting only — the control is derived, + * not configured. + * @param known every credential name the operator's store is known to export. Under + * deny-list/deny-by-default every name here that is NOT also in {@link #allow} is + * overlaid with a non-secret sentinel value. Under allow-list this list is + * reporting only. + * @param sshAuthSock whether the member may inherit {@code SSH_AUTH_SOCK} under the allow-list + * policy ({@code "allow"}) or must have it blanked ({@code "block"}, the default). + * This is a DECISION, never a default: {@code SSH_AUTH_SOCK} is a handle to the + * operator's ssh-agent, and a member holding it can sign with the operator's own + * keys — but it appears in no secret file and is credential-shaped like nothing on + * any list, which is why three earlier tickets missed it (gitea #110 / CB-607). + * Blocking it breaks git over SSH inside the member; allow it only when members do + * not need to authenticate as the operator over SSH. Ignored under deny-list / + * deny-by-default, which never touch the name. */ @JsonIgnoreProperties(ignoreUnknown = true) - public record MemberCredentials(String policy, List allow, List known) { + public record MemberCredentials(String policy, List allow, List known, + String sshAuthSock) { - /** The only policy this build understands: block every {@code known} name not in {@code allow}. */ + /** Default policy: block every {@code known} name not in {@code allow}, via the env overlay. */ public static final String POLICY_DENY_BY_DEFAULT = "deny-by-default"; + /** Alias of {@link #POLICY_DENY_BY_DEFAULT}, spelled the way CB-633 names the two policies. */ + public static final String POLICY_DENY_LIST = "deny-list"; + + /** + * CB-633: derive the kept-name set from what the launcher itself injects, generate a + * per-spawn ZDOTDIR whose {@code .zlogin} blanks every exported variable not on it AFTER the + * pane's login shell has finished sourcing the operator's chain. + */ + public static final String POLICY_ALLOW_LIST = "allow-list"; + + /** The pre-CB-633 three-field form — {@code sshAuthSock} defaults to blocked. */ + public MemberCredentials(String policy, List allow, List known) { + this(policy, allow, known, null); + } + public MemberCredentials { - policy = (policy == null || policy.isBlank()) ? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(); + String normalizedPolicy = (policy == null || policy.isBlank()) + ? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(); + // deny-list is an alias of deny-by-default, not a third behaviour — normalize to one + // spelling so every isDenyList()-style check has one value to compare against. + policy = POLICY_DENY_LIST.equals(normalizedPolicy) ? POLICY_DENY_BY_DEFAULT : normalizedPolicy; allow = allow == null ? List.of() : List.copyOf(allow); known = known == null ? List.of() : List.copyOf(known); + sshAuthSock = (sshAuthSock != null && "allow".equalsIgnoreCase(sshAuthSock.trim())) + ? "allow" : "block"; + } + + /** True when this block selects the CB-633 derived-allow-list policy. */ + public boolean isAllowList() { + return POLICY_ALLOW_LIST.equals(policy); + } + + /** True when {@code SSH_AUTH_SOCK} may pass through under the allow-list policy. Default: no. */ + public boolean sshAuthSockAllowed() { + return "allow".equals(sshAuthSock); } /** {@link #allow} as a set, for membership checks. */ @@ -1487,9 +1544,15 @@ public record FleetConfig( } } - /** The member-credential policies this build understands — {@link MemberCredentials#policy()}'s only valid value. */ + /** + * The member-credential policies this build understands — {@link MemberCredentials#policy()}'s + * only valid values. Checked against the RAW yaml text (before {@link MemberCredentials}'s + * compact constructor normalizes {@code deny-list} onto {@code deny-by-default}), so the alias + * is listed explicitly. + */ private static final Set KNOWN_MEMBER_CREDENTIALS_POLICIES = - Set.of(MemberCredentials.POLICY_DENY_BY_DEFAULT); + Set.of(MemberCredentials.POLICY_DENY_BY_DEFAULT, MemberCredentials.POLICY_DENY_LIST, + MemberCredentials.POLICY_ALLOW_LIST); /** * Reject a {@code memberCredentials.policy} that is not {@link #KNOWN_MEMBER_CREDENTIALS_POLICIES} @@ -1602,6 +1665,9 @@ public record FleetConfig( // is deliberately not pre-populated with a Java-side name list (that would just reintroduce // the hardcoded-list defect this record replaces); the block must be configured in // bridged.yaml to protect anything. See fleetd.example.yaml's memberCredentials: comment. + // CB-633: policy stays deny-by-default here — the allow-list scrub is opt-in, because it is + // stricter than today's behaviour (it blanks every non-derived name, not just known ones) + // and an upgrade must not change what a running deployment's members inherit. MemberCredentials mc = memberCredentials != null ? memberCredentials : new MemberCredentials(null, List.of(), List.of()); return new FleetConfig(b, herdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs, diff --git a/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java b/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java new file mode 100644 index 0000000..5cc4812 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -0,0 +1,193 @@ +package dev.ltms.fleet.member; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; + +/** + * CB-633: generates the per-spawn {@code ZDOTDIR} directory whose startup files enforce + * {@code memberCredentials.policy: allow-list}. + * + *

The seam: a login interactive zsh sources {@code $ZDOTDIR/.zshenv}, then {@code .zprofile}, + * then {@code .zshrc}, then {@code .zlogin} — in that order, LAST first-named-last. The operator's + * whole chain ({@code ~/.zshrc} → secret store) runs inside the first three, so a scrub placed in + * the generated {@code .zlogin} runs after everything the operator sourced, and nothing later can + * re-export over it. This is the property CB-596's env-overlay control lacked: herdr applies that + * overlay BEFORE the login shell starts, so any sourced file can undo it — and did. + * + *

Each generated file sources its {@code $HOME} counterpart FIRST, so {@code PATH} and every + * toolchain binary still resolve exactly as the operator configured them; only afterwards does + * {@code .zlogin} run the scrub: every EXPORTED variable not on the derived allow-list is re-exported + * blank. Blank, not credential-shaped-pattern-filtered: a pattern list ({@code *TOKEN*}, …) is an + * enumeration and misses what it did not think of — a username is the other half of a credential and + * is shaped like none. Credential-SHAPED names among the blanked set go to the WARN log only, + * never to the control. + * + *

The scrub also writes {@code scrub-report.txt} into its own directory: one {@code allowed N of + * M} line (N = exports left untouched, M = exports present when the scrub ran), then the blanked + * NAMES — never values. The launcher reads this back at teardown and logs it, because a blocked + * count next to an unknown denominator is not a finding. + */ +public final class EnvAllowListScrub { + + /** Name of the report file written into the generated directory by the scrub itself. */ + static final String REPORT_FILE = "scrub-report.txt"; + + private EnvAllowListScrub() { + } + + /** + * A parsed {@code scrub-report.txt}: how many exported variables existed when the scrub ran, + * how many were left untouched (allowed), and the NAMES that were blanked. Values never appear. + */ + record ScrubReport(int allowed, int total, List blanked) { + } + + /** + * Create a fresh ZDOTDIR directory under {@code parentDir} holding the four zsh startup files. + * Every file (and the directory) registers {@code deleteOnExit}, next to the existing per-spawn + * charter/config temp cleanup; the launcher additionally deletes eagerly at pane release. + * + * @param allowedNames the DERIVED allow-list — exact variable names that must survive the scrub + * @return the directory path (to be passed as the pane's {@code ZDOTDIR}) + * @throws UncheckedIOException when the directory or any file cannot be written — a spawn whose + * protection cannot even be materialized must fail loudly rather + * than start unprotected + */ + public static Path generate(Path parentDir, Set allowedNames) { + try { + Path dir = Files.createTempDirectory(parentDir, "bridged-zdotdir-"); + dir.toFile().deleteOnExit(); + write(dir, ".zshenv", homeSourcingFile(".zshenv")); + write(dir, ".zprofile", homeSourcingFile(".zprofile")); + write(dir, ".zshrc", homeSourcingFile(".zshrc")); + write(dir, ".zlogin", zloginScript(allowedNames)); + return dir; + } catch (IOException e) { + throw new UncheckedIOException("cannot generate ZDOTDIR scrub files under " + parentDir, e); + } + } + + /** One operator-sourcing startup file: source the {@code $HOME} counterpart, change nothing else. */ + private static String homeSourcingFile(String name) { + return """ + # generated by fleetd (CB-633 memberCredentials policy=allow-list) — do not edit. + # Source the operator's own %s first, so PATH and the agent binaries resolve as usual. + [ -r "$HOME/%s" ] && source "$HOME/%s" + """.formatted(name, name, name); + } + + /** + * The generated {@code .zlogin}: source the operator's own {@code ~/.zlogin}, then run the scrub. + * Package-private so tests can assert on the exact script handed to zsh — the artefact here IS a + * shell file, and a test that checks only the Java string assembly proves nothing about whether + * zsh accepts it. + */ + static String zloginScript(Set allowedNames) { + StringBuilder names = new StringBuilder(); + for (String n : allowedNames.stream().sorted().toList()) { + if (names.length() > 0) { + names.append(' '); + } + // Names are validated against [A-Za-z_][A-Za-z0-9_]* before they get here; single quotes + // keep even a non-conforming name inert rather than executable. + names.append('\'').append(n.replace("'", "")).append('\''); + } + return """ + # generated by fleetd (CB-633 memberCredentials policy=allow-list) — do not edit. + # Runs LAST in the login-shell order, after everything the operator sourced. + [ -r "$HOME/.zlogin" ] && source "$HOME/.zlogin" + + typeset -A _cb633_allowed + for _cb633_n in %s; do _cb633_allowed[$_cb633_n]=1; done + + # Enumerate EXPORTED variable NAMES from `env` itself. Deliberately NOT the special + # `parameters` assoc: its subscript is evaluated arithmetically on this host's zsh + # and blows up on some names ("bad math expression"). Names not matching the + # identifier pattern (junk from multi-line values) are skipped, never scrubbed. + typeset -a _cb633_names + _cb633_names=("${(@f)$(command env | command cut -d= -f1)}") + typeset -a _cb633_blank + _cb633_blank=() + integer _cb633_total=0 + for _cb633_n in "${_cb633_names[@]}"; do + [[ "$_cb633_n" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]] || continue + (( _cb633_total += 1 )) + [[ -n "${_cb633_allowed[$_cb633_n]-}" ]] && continue + case "$_cb633_n" in %s) continue ;; esac + _cb633_blank+=("$_cb633_n") + done + + { for _cb633_n in "${_cb633_blank[@]}"; do export "$_cb633_n="; done; } 2>/dev/null + + integer _cb633_kept=$(( _cb633_total - ${#_cb633_blank} )) + { + print -r -- "allowed $_cb633_kept of $_cb633_total" + for _cb633_n in "${_cb633_blank[@]}"; do print -r -- "$_cb633_n"; done + } > "$ZDOTDIR/%s" 2>/dev/null + + unset _cb633_allowed _cb633_names _cb633_blank _cb633_n _cb633_total _cb633_kept + """.formatted(names, MemberEnvAllowList.zshCasePattern(), REPORT_FILE); + } + + private static void write(Path dir, String fileName, String content) throws IOException { + Path file = dir.resolve(fileName); + Files.writeString(file, content); + file.toFile().deleteOnExit(); + } + + /** + * Read and parse {@link #REPORT_FILE} out of a generated ZDOTDIR directory. Returns {@code null} + * when absent or unreadable (the pane may have been torn down before its login shell ever got to + * the scrub) — callers treat that as "no measurement available", never as success. + */ + static ScrubReport readReport(Path zdotdir) { + Path report = zdotdir.resolve(REPORT_FILE); + if (!Files.isRegularFile(report)) { + return null; + } + try { + List lines = Files.readAllLines(report); + if (lines.isEmpty() || !lines.getFirst().startsWith("allowed ")) { + return null; + } + String[] parts = lines.getFirst().substring("allowed ".length()).trim().split("\\s+"); + if (parts.length != 3 || !"of".equals(parts[1])) { + return null; + } + List blanked = new ArrayList<>(); + for (int i = 1; i < lines.size(); i++) { + if (!lines.get(i).isBlank()) { + blanked.add(lines.get(i)); + } + } + return new ScrubReport(Integer.parseInt(parts[0]), Integer.parseInt(parts[2]), + List.copyOf(blanked)); + } catch (IOException | NumberFormatException e) { + return null; + } + } + + /** Best-effort recursive delete; failures are swallowed — JVM-exit cleanup is the backstop. */ + static void deleteRecursively(Path dir) { + if (dir == null || !Files.exists(dir)) { + return; + } + try (Stream walk = Files.walk(dir)) { + walk.sorted(java.util.Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best effort — deleteOnExit retries at JVM shutdown + } + }); + } catch (IOException ignored) { + // same + } + } +} diff --git a/bridged/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java index c532b54..115ce7d 100644 --- a/bridged/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -156,6 +156,20 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { private final ConcurrentMap paneByAgentId = new ConcurrentHashMap<>(); private final AtomicBoolean resetUnsupportedLogged = new AtomicBoolean(); + /** + * CB-633: the per-spawn ZDOTDIR directory generated for a pane under + * {@code memberCredentials.policy: allow-list}, keyed by herdr pane id so every teardown exit + * ({@link #stop} is reached from explicit DELETE, orphan reap, and the spawn-readiness gate + * timeout alike) can read the scrub's own report and then remove the directory. A pane that + * never reaches {@code stop} (spawn failure) leaks its directory only until JVM exit, where the + * generator's {@code deleteOnExit} hooks are the backstop — the same cleanup shape the existing + * charter/config temp files use. + */ + private final ConcurrentMap zdotdirByPane = new ConcurrentHashMap<>(); + + /** Guards {@link #warnNonZsh} to one WARN per launcher instance, not one per spawn. */ + private final AtomicBoolean nonZshShellWarned = new AtomicBoolean(); + /** * @param namePrefix label prefix for this peer kind (drives naming and reap) * @param agents herdr agent control (start, status, close) @@ -420,9 +434,16 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { try { Launch launch = buildLaunch(cfg, new LaunchSpec(sessionName, resumeSessionId, role, charter, roleCharter, replyCharter, cwd)); + // CB-633: applied AFTER buildLaunch so the generated scrub's allow-list can also cover + // the exact env-map keys this launch injects (ANTHROPIC_*, OPENCODE_CONFIG, GITEA_TOKEN, …) + // — anything the daemon deliberately sets must survive its own control. + Path zdotdir = applyEnvironmentAllowListPolicy(cfg, launch); Agent agent = cfg.tabPlacement() ? spawnInTab(cfg, launch.env(), launch.argv(), cwd, role, liveFleet) : spawnAsPane(cfg, launch.env(), launch.argv(), cwd, charter); + if (zdotdir != null) { + zdotdirByPane.put(agent.paneId(), zdotdir); + } logCharterReceipt(receipt, true); return new Spawned(agent, launch.agentSessionId(), receipt); } catch (RuntimeException e) { @@ -774,6 +795,14 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { log.debug("not closing tab {} — it holds {} panes (not a dedicated peer tab)", loc.tabId(), loc.tabPaneCount()); } + // CB-633: log this pane's allowed-N-of-M scrub report, then remove the generated ZDOTDIR. + // Last in, best-effort — a failure here must not mask a real teardown failure above. + try { + releaseZdotdir(paneId); + } catch (RuntimeException e) { + log.warn("memberCredentials allow-list: releasing ZDOTDIR for pane {} failed: {}", + paneId, e.getMessage()); + } } /** Whether any configured profile places peers in their own tab (so tabs may need cleanup). */ @@ -957,16 +986,120 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * one whose {@code known} list is empty — shadows nothing. This is a real, config-driven gap * (see {@link FleetConfig.MemberCredentials}'s javadoc), not a safe default: deny-by-default * only defends names the operator has actually enumerated in {@code known}. + * + *

CB-633: under {@code policy: allow-list} this overlay is NOT the control anymore — it is + * applied before the login shell runs and a sourced file can (and did) undo it. The control is + * the ZDOTDIR scrub ({@link #applyEnvironmentAllowListPolicy}); {@code known}/{@code allow} + * remain as reporting only via {@link #logCredentialGap}. */ private void applyMemberCredentialPolicy(Map workerEnv) { FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get(); if (creds == null) { return; } + if (!creds.isAllowList()) { + overlayBlockedCredentials(workerEnv, creds); + } + logCredentialGap(creds); + } + + /** 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()) { workerEnv.put(name, BLOCKED_CREDENTIAL_SENTINEL); } - logCredentialGap(creds); + } + + /** + * CB-633: under {@code memberCredentials.policy: allow-list}, generate the per-spawn ZDOTDIR + * directory whose {@code .zlogin} blanks every exported variable not on the DERIVED allow-list — + * running AFTER the pane's login shell has finished sourcing the operator's chain, which is what + * no pre-shell env overlay can achieve. Mutates {@code launch.env()} to carry + * {@code ZDOTDIR=

}, so both placement paths ({@link #spawnInTab}, {@link #spawnAsPane}) + * pass it through {@code tab.create}/{@code pane.split}. Returns the directory for teardown + * registration, or {@code null} when the policy does not apply. + * + *

The allow-list handed to the generator is the derived profile set ({@link + * MemberEnvAllowList#derive}) UNIONed with the exact keys of THIS launch's env map — names the + * daemon itself injects must survive its own control. {@code SSH_AUTH_SOCK} is added ONLY when + * the config explicitly allows it; by default it is absent, so the scrub blanks it like any + * other non-derived name. + */ + private Path applyEnvironmentAllowListPolicy(FleetConfig.Profile cfg, Launch launch) { + FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get(); + if (creds == null || !creds.isAllowList()) { + return null; + } + String loginShell = resolveEnv("SHELL"); + boolean zsh = loginShell != null && (loginShell.endsWith("/zsh") || loginShell.equals("zsh")); + 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. + warnNonZsh(loginShell); + overlayBlockedCredentials(launch.env(), creds); + logCredentialGap(creds); + return null; + } + Set allowed = new java.util.TreeSet<>(MemberEnvAllowList.derive(profiles.values())); + 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()); + Path dir = EnvAllowListScrub.generate(Path.of(System.getProperty("java.io.tmpdir")), 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", + cfg.profile(), dir.getFileName(), allowed.size()); + return dir; + } + + /** The operator ssh-agent handle — kept ONLY by explicit config decision, never by default. */ + private static final String SSH_AUTH_SOCK = "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. + */ + private void warnNonZsh(String shell) { + 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.", + shell == null ? "" : shell); + } + } + + /** + * CB-633 teardown half: read the pane's scrub report (the denominator report the generated + * {@code .zlogin} wrote) and delete the directory. Called from {@link #stop}, which is the one + * funnel every teardown exit already goes through. Best-effort throughout: a missing report is + * logged at debug, never an error — the pane may be gone before its shell reached the scrub. + */ + private void releaseZdotdir(String paneId) { + Path dir = zdotdirByPane.remove(paneId); + if (dir == null) { + return; + } + EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(dir); + if (report == null) { + log.debug("memberCredentials allow-list: pane {} left no scrub report", paneId); + } else { + log.info("memberCredentials allow-list: pane {} allowed {} of {} environment variables", + paneId, report.allowed(), report.total()); + List shaped = report.blanked().stream() + .filter(name -> CREDENTIAL_SHAPED_NAME.matcher(name).matches()) + .toList(); + if (!shaped.isEmpty()) { + log.warn("memberCredentials allow-list: pane {} blanked credential-shaped variable(s) " + + "{} — confirm none of them was something a member legitimately needed", + paneId, shaped); + } + } + EnvAllowListScrub.deleteRecursively(dir); } /** Credential-shaped env var name heuristic for {@link #logCredentialGap} — case-insensitive. */ diff --git a/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java b/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java new file mode 100644 index 0000000..9e97562 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java @@ -0,0 +1,108 @@ +package dev.ltms.fleet.member; + +import dev.ltms.fleet.config.FleetConfig; + +import java.util.Collection; +import java.util.Set; +import java.util.TreeSet; + +/** + * CB-633: the set of environment variable NAMES a spawned member is allowed to keep under + * {@code memberCredentials.policy: allow-list} — DERIVED from what the launcher itself injects, + * never hand-typed. + * + *

A hand-typed allow-list is the defect this class exists to prevent: a name an operator forgets + * to type is a credential that passes through to every member, and a profile added to config later + * would silently break spawns whose scrub did not know its names. Derivation closes both ends. The + * kept-name set is the union of: + * + *

+ * + *

Because the union spans EVERY profile (not just the one spawning), adding a new profile can + * only ever widen the list — it cannot break another spawn's scrub. And because the launcher also + * unions in the exact keys of each spawn's own env map at generation time (see {@code + * HerdrPeerLauncher}), anything the daemon deliberately injects for THIS spawn survives its own + * control. + * + *

{@code SSH_AUTH_SOCK} is deliberately NOT here. It is a handle to the operator's ssh-agent — a + * member holding it can sign with the operator's keys — so keeping it is a config decision + * ({@code memberCredentials.sshAuthSock: allow}), not a derivation default. + */ +public final class MemberEnvAllowList { + + /** + * Names that are not credentials and that a login shell or agent binary genuinely needs. + * + *

Deliberately conservative beyond the ticket's named set: {@code ZDOTDIR} must survive or + * every later sub-shell loses the scrub; {@code BRIDGED_MEMBER} is the daemon's own marker; + * {@code GITEA_TOKEN}/{@code GITEA_HOST} are what {@code applyGitToken} injects by literal name; + * the {@code ANTHROPIC_*}/{@code CLAUDE_CONFIG_DIR}/{@code OPENCODE_CONFIG} names are what the + * adapters inject by literal name (they are also re-added per-spawn from the env map itself — + * listing them here keeps the derived set self-contained for tests and reporting); {@code + * JAVA_HOME} and the {@code XDG_*} roots are toolchain locations, not secrets. Everything else a + * member needs must arrive via a profile's {@code env:}, which lands on this set automatically. + */ + public static final Set INFRASTRUCTURE_PASSTHROUGH = Set.of( + "PATH", "HOME", "SHELL", "TERM", "LANG", "TMPDIR", + "USER", "LOGNAME", "PWD", "SHLVL", "EDITOR", "PAGER", + "_", + "ZDOTDIR", "BRIDGED_MEMBER", + "GITEA_TOKEN", "GITEA_HOST", + "ANTHROPIC_BASE_URL", "ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_MODEL", + "CLAUDE_CONFIG_DIR", "OPENCODE_CONFIG", + "JAVA_HOME", + "XDG_CONFIG_HOME", "XDG_DATA_HOME", "XDG_CACHE_HOME", "XDG_STATE_HOME"); + + /** Locale-category prefix kept as infrastructure ({@code LC_ALL}, {@code LC_CTYPE}, …). */ + private static final String INFRASTRUCTURE_NAME_PREFIX = "LC_"; + + private MemberEnvAllowList() { + } + + /** + * Derive the allowed NAME set from the given profiles plus {@link #INFRASTRUCTURE_PASSTHROUGH}. + * Deterministic (sorted) so generated scrub files are diffable run-to-run. + */ + public static Set derive(Collection profiles) { + Set derived = new TreeSet<>(INFRASTRUCTURE_PASSTHROUGH); + if (profiles != null) { + for (FleetConfig.Profile p : profiles) { + addIfPresent(derived, p.gitTokenEnv()); + addIfPresent(derived, p.gitHostEnv()); + addIfPresent(derived, p.tokenEnv()); + if (p.env() != null) { + derived.addAll(p.env().keySet()); + } + } + } + return Set.copyOf(derived); + } + + /** + * 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 + * the generated script's {@code case} pattern, which is written from this constant's value. + */ + public static boolean keeps(Set allowedNames, String name) { + return allowedNames.contains(name) || name.startsWith(INFRASTRUCTURE_NAME_PREFIX); + } + + /** The prefix rule as a zsh {@code case} pattern, so the script and Java cannot drift apart. */ + public static String zshCasePattern() { + return INFRASTRUCTURE_NAME_PREFIX + "*"; + } + + private static void addIfPresent(Set into, String name) { + if (name != null && !name.isBlank()) { + into.add(name); + } + } +} diff --git a/bridged/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java b/bridged/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java index f7ff0b4..d1cd9dc 100644 --- a/bridged/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java +++ b/bridged/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java @@ -1513,6 +1513,66 @@ class FleetConfigTest { "blockedSet is known minus allow"); } + /** + * CB-633: {@code policy: allow-list} binds, is case-insensitive, and flips the block into + * derived-scrub mode ({@link FleetConfig.MemberCredentials#isAllowList()}). + */ + @Test + void allowListPolicyBinds(@TempDir Path dir) throws Exception { + Path f = dir.resolve("member-credentials-allow-list.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + memberCredentials: + policy: ALLOW-LIST + """); + + FleetConfig.MemberCredentials mc = FleetConfig.load(f).memberCredentials(); + assertEquals(FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, mc.policy()); + assertTrue(mc.isAllowList(), "allow-list must select the CB-633 derived-scrub policy"); + } + + /** + * CB-633: {@code deny-list} is an accepted spelling of today's behaviour, normalized onto the + * one canonical value — an operator upgrading from prose that says "deny-list" must not be told + * their policy is unrecognized. + */ + @Test + void denyListIsAnAcceptedAliasOfDenyByDefault(@TempDir Path dir) throws Exception { + Path f = dir.resolve("member-credentials-deny-list.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + memberCredentials: + policy: deny-list + known: + - GITEA_ACCESS_TOKEN + """); + + FleetConfig.MemberCredentials mc = FleetConfig.load(f).memberCredentials(); + assertFalse(mc.isAllowList()); + assertEquals(FleetConfig.MemberCredentials.POLICY_DENY_BY_DEFAULT, mc.policy(), + "deny-list normalizes onto the canonical deny-by-default value"); + } + + /** + * CB-633: {@code SSH_AUTH_SOCK} is a decision, never a default — absent, blank, or misspelled, + * it stays BLOCKED; only the literal "allow" (any case) passes it through. A typo like "alow" + * failing safe here is the whole point of making it a knob. + */ + @Test + void sshAuthSockDefaultsToBlockedAndOnlyExplicitAllowUnblocksIt() { + assertTrue(new FleetConfig.MemberCredentials("allow-list", List.of(), List.of()).sshAuthSock().equals("block"), + "absent knob blocks SSH_AUTH_SOCK"); + assertFalse(new FleetConfig.MemberCredentials("allow-list", List.of(), List.of()).sshAuthSockAllowed()); + assertFalse(new FleetConfig.MemberCredentials(null, null, null, "").sshAuthSockAllowed(), + "blank knob blocks SSH_AUTH_SOCK"); + assertFalse(new FleetConfig.MemberCredentials(null, null, null, "alow").sshAuthSockAllowed(), + "a misspelled value fails SAFE, not open"); + assertTrue(new FleetConfig.MemberCredentials(null, null, null, "ALLOW").sshAuthSockAllowed(), + "the literal allow (case-insensitive) unblocks SSH_AUTH_SOCK"); + } + /** * CB-596: omitting {@code memberCredentials:} entirely must NOT crash a reader that assumes a * non-null block (the same "fill in nested defaults" contract every other structural field diff --git a/bridged/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java b/bridged/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java new file mode 100644 index 0000000..46475d9 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java @@ -0,0 +1,153 @@ +package dev.ltms.fleet.member; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * CB-633: the artefact here is a shell file handed to ANOTHER PROGRAM — so the only test that + * proves anything is one that runs that program. A unit test on {@link EnvAllowListScrub}'s string + * assembly proves nothing about zsh; this repo has shipped a green suite before whose tests checked + * argv we build and none ran the binary that has to accept it. + * + *

This test starts a REAL login interactive zsh from a CLEAN parent ({@code env -i}), once with + * the generated ZDOTDIR and once without (the baseline), and asserts the surviving exported NAME set + * EQUALS the allow-list intersection of the baseline — equality, not "these names are blocked". A + * blocked-name list can only check names somebody already thought of; that is exactly the failure + * being fixed. Only NAMES are compared — never values. + * + *

The scrub run sources this operator's real {@code ~/.zshenv}/~/.zprofile/~/.zshrc/~/.zlogin} + * chain, so it skips cleanly (JUnit {@code assumeTrue}) on a machine with no /bin/zsh or no real + * shell rc files rather than failing there. + */ +class EnvAllowListScrubTest { + + private static final Path ZSH = Path.of("/bin/zsh"); + + /** Env var names appearing in command output; anything else (prompts, wrapped lines) is noise. */ + private static final Pattern ENV_NAME = Pattern.compile("^([A-Za-z_][A-Za-z0-9_]*)$"); + + /** + * The equality test. Expected survivors = baseline exports ∩ allowed — i.e. every survivor is + * allowed AND every allowed name that existed survives. The operator's own secret-store exports + * (~30 names on this host) are the decoys: none is on the derived list, so every one must be + * gone from the scrub run. + */ + @Test + void scrubbedLoginShellSurvivorsEqualTheDerivedAllowList(@TempDir Path tmp) throws Exception { + assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here"); + Path homeZshrc = Path.of(System.getProperty("user.home"), ".zshrc"); + assumeTrue(Files.exists(homeZshrc), "$HOME/.zshrc does not exist — no real login chain to test against"); + + // Derived shape, zero profiles: infrastructure + LC_* rule. SSH_AUTH_SOCK deliberately NOT + // included — blocked by default is the decision under test. + Set allowed = MemberEnvAllowList.derive(List.of()); + Path zdotdir = EnvAllowListScrub.generate(tmp, allowed); + + Map cleanParent = Map.of( + "HOME", System.getProperty("user.home"), + "PATH", "/usr/bin:/bin", + "SHELL", "/bin/zsh", + "USER", System.getProperty("user.name", "nobody"), + "TMPDIR", tmp.toString()); + + Set baseline = exportedNamesFromCleanParent(cleanParent, null); + Set scrubbed = exportedNamesFromCleanParent(cleanParent, zdotdir); + + // The scrub RUN itself carries ZDOTDIR (the harness set it; it is infrastructure and MUST + // survive, or every later login shell loses the scrub) — so the expected set starts from + // baseline plus that one name. + Set expected = new TreeSet<>(); + for (String name : baseline) { + if (MemberEnvAllowList.keeps(allowed, name)) { + expected.add(name); + } + } + assertTrue(MemberEnvAllowList.keeps(allowed, "ZDOTDIR")); + expected.add("ZDOTDIR"); + assertEquals(expected, scrubbed, + "surviving exported names must EQUAL baseline ∩ allow-list — a survivor outside the " + + "list is a leak; a missing allowed name means the scrub broke something it " + + "should have kept. Names only, values never printed."); + } + + /** The scrub writes its denominator report next to itself; names only, parseable. */ + @Test + void scrubWritesAnAllowedNofMReport(@TempDir Path tmp) throws Exception { + Set allowed = MemberEnvAllowList.derive(List.of()); + Path zdotdir = EnvAllowListScrub.generate(tmp, allowed); + + Map cleanParent = Map.of( + "HOME", System.getProperty("user.home"), + "PATH", "/usr/bin:/bin", + "SHELL", "/bin/zsh"); + exportedNamesFromCleanParent(cleanParent, zdotdir); // runs the login shell → runs the scrub + + EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir); + assertNotNull(report, "a completed login shell must leave a report behind"); + assertTrue(report.allowed() >= 0 && report.total() >= report.allowed(), + "allowed N of M with N <= M — the denominator is always reported"); + } + + /** Report parsing is lenient: absent file → null (no measurement), not an exception. */ + @Test + void readReportReturnsNullForADirectoryWithoutOne(@TempDir Path dir) { + assertNull(EnvAllowListScrub.readReport(dir)); + } + + /** + * Run {@code /bin/zsh -l -i} from a clean parent and return the NAMES it has exported by prompt + * time. With {@code zdotdir} non-null, {@code ZDOTDIR} points at a generated scrub directory, so + * the login chain ends in CB-633's scrub; null gives the un-scrubbed baseline. + */ + private static Set exportedNamesFromCleanParent(Map cleanParent, + Path zdotdir) throws IOException, InterruptedException { + ProcessBuilder pb = new ProcessBuilder("/bin/zsh", "-l", "-i"); + pb.environment().clear(); + pb.environment().putAll(cleanParent); + if (zdotdir != null) { + pb.environment().put("ZDOTDIR", zdotdir.toAbsolutePath().toString()); + } + pb.redirectError(ProcessBuilder.Redirect.DISCARD); // prompts and rc chatter, never data + + Process zsh = pb.start(); + // Names of exports whose VALUE is still non-empty. A blanked variable stays EXPORTED with + // an empty value ("NAME=") — that is the control working, not surviving — so plain + // `env | cut -d= -f1` would wrongly count blanked names as survivors. + String probeScript = "command env | awk -F= '/^[A-Za-z_][A-Za-z0-9_]*=/ && length($0) > " + + "length($1)+1 { print $1 }' | command sort -u\nexit\n"; + zsh.getOutputStream().write(probeScript.getBytes(StandardCharsets.UTF_8)); + zsh.getOutputStream().flush(); + + String stdout = new String(zsh.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertTrue(zsh.waitFor(60, java.util.concurrent.TimeUnit.SECONDS), + "the probe login shell did not exit within 60s"); + assertTrue(zsh.exitValue() == 0, "probe zsh exited non-zero — see test failure, values never printed"); + + Set names = new HashSet<>(); + for (String line : stdout.split("\n")) { + Matcher m = ENV_NAME.matcher(line.trim()); + if (m.matches()) { + names.add(m.group(1)); + } + } + return names; + } +} diff --git a/bridged/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java b/bridged/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java new file mode 100644 index 0000000..bb7f983 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java @@ -0,0 +1,105 @@ +package dev.ltms.fleet.member; + +import dev.ltms.fleet.config.FleetConfig; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CB-633: the allow-list a member's environment scrub enforces is DERIVED from what the launcher + * itself injects — never hand-typed. These tests pin the derivation: adding a profile can only + * widen the set (never break another spawn), and a name no profile references is not on it. + */ +class MemberEnvAllowListTest { + + /** The 18-arg Profile ctor (back-compat + CB-511 {@code env} passthrough). */ + private static FleetConfig.Profile profile(String name, String tokenEnv, + String gitTokenEnv, String gitHostEnv, + Map env) { + return new FleetConfig.Profile(name, "http://gx00.gw:8000", null, null, tokenEnv, + List.of("claude"), null, null, null, null, null, null, + gitTokenEnv, gitHostEnv, FleetConfig.Profile.KIND_CLAUDE_CODE, env, 1.0f, 5); + } + + @Test + void everyKeyOfEveryProfileEnvMapLandsOnTheDerivedList() { + FleetConfig.Profile p = profile("p", null, null, null, + Map.of("MY_TOOL_ENDPOINT", "http://10.0.0.1", "MY_TOOL_OPTION", "x")); + + Set derived = MemberEnvAllowList.derive(List.of(p)); + + assertTrue(derived.contains("MY_TOOL_ENDPOINT")); + assertTrue(derived.contains("MY_TOOL_OPTION")); + } + + @Test + void everyProfileTokenEnvGitTokenEnvAndGitHostEnvNameLandsOnTheDerivedList() { + FleetConfig.Profile p = profile("p", "GATEWAY_TOKEN_FOR_P", + "WORKER_GITEA_TOKEN", "MY_GITEA_HOST", Map.of()); + + Set derived = MemberEnvAllowList.derive(List.of(p)); + + assertTrue(derived.contains("GATEWAY_TOKEN_FOR_P"), "tokenEnv is a variable NAME held in config"); + assertTrue(derived.contains("WORKER_GITEA_TOKEN")); + assertTrue(derived.contains("MY_GITEA_HOST")); + } + + @Test + void aNameNoProfileReferencesIsNotOnTheDerivedList() { + Set derived = MemberEnvAllowList.derive(List.of( + profile("p", "TOKEN_A", null, null, Map.of("KEY_A", "v")))); + + assertFalse(derived.contains("SOME_SECRET_NOBODY_CONFIGURED"), + "a hand-typed-feeling name must not appear by magic"); + assertFalse(derived.contains("CONFLUENCE_USERNAME"), + "the exact class of name pattern filters missed"); + } + + @Test + void infrastructurePassthroughNamesAreAlwaysOnTheDerivedList() { + for (String infra : new String[]{"PATH", "HOME", "TERM", "TMPDIR", "SHLVL", "ZDOTDIR"}) { + assertTrue(MemberEnvAllowList.INFRASTRUCTURE_PASSTHROUGH.contains(infra), + infra + " is infrastructure, not a credential"); + assertTrue(MemberEnvAllowList.derive(List.of()).contains(infra), + "derived list holds infrastructure even with zero profiles"); + } + } + + /** + * The property the ticket hangs the design on: ADDING a profile only ever widens the set, so a + * new profile in bridged.yaml cannot break an existing spawn's scrub. + */ + @Test + void addingAProfileOnlyWidensTheDerivedSetNeverShrinksIt() { + FleetConfig.Profile first = profile("first", "TOKEN_FIRST", null, null, Map.of("FIRST_KEY", "v")); + Set before = MemberEnvAllowList.derive(List.of(first)); + + FleetConfig.Profile second = profile("second", "TOKEN_SECOND", "GIT_TOK", null, + Map.of("SECOND_KEY", "v")); + Set after = MemberEnvAllowList.derive(List.of(first, second)); + + assertTrue(after.containsAll(before), "widening only"); + assertTrue(after.containsAll(Set.of("TOKEN_SECOND", "GIT_TOK", "SECOND_KEY"))); + } + + /** {@code LC_*} categories are infrastructure by prefix; everything else needs an exact match. */ + @Test + void keepsMatchesExactlyPlusTheLocalePrefixRule() { + Set derived = MemberEnvAllowList.derive(List.of()); + + assertTrue(MemberEnvAllowList.keeps(derived, "LC_FOO"), "prefix rule lives here, not in the caller"); + assertFalse(MemberEnvAllowList.keeps(derived, "LCD_VAR"), + "a name that merely STARTS with the prefix letters is not LC_*"); + assertTrue(MemberEnvAllowList.keeps(Set.of("MINE"), "MINE")); + assertFalse(MemberEnvAllowList.keeps(Set.of(), "SSH_AUTH_SOCK"), + "the ssh-agent handle is kept ONLY by explicit config decision, never by this rule"); + assertEquals("LC_*", MemberEnvAllowList.zshCasePattern(), + "the generated script's case pattern and this rule must not drift apart"); + } +}