CB-633: memberCredentials policy=allow-list — derived ZDOTDIR env scrub
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 "
|
||||
|
||||
@@ -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.
|
||||
* <p><b>CB-633: allow-list.</b> 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<String> allow, List<String> known) {
|
||||
public record MemberCredentials(String policy, List<String> allow, List<String> 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<String> allow, List<String> 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<String> 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,
|
||||
|
||||
@@ -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}.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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<String> 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<String> 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<String> 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<String> 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<String> 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<Path> 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
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -156,6 +156,20 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
private final ConcurrentMap<String, String> 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<String, Path> 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}.
|
||||
*
|
||||
* <p>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<String, String> 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<String, String> 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=<dir>}, 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.
|
||||
*
|
||||
* <p>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<String> 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 ? "<unset>" : 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<String> 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. */
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>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:
|
||||
*
|
||||
* <ul>
|
||||
* <li>every configured {@link FleetConfig.Profile profile}'s {@code gitTokenEnv},
|
||||
* {@code gitHostEnv}, and {@code tokenEnv} values — these are variable <em>names</em> held in
|
||||
* config, and the launcher reads their values out of exactly these variables;</li>
|
||||
* <li>every key of every profile's {@code env:} map — anything the operator routes into a pane on
|
||||
* purpose;</li>
|
||||
* <li>{@link #INFRASTRUCTURE_PASSTHROUGH} — names that are not credentials at all but that a shell
|
||||
* or the agent binary genuinely needs to function.</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>{@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.
|
||||
*
|
||||
* <p>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<String> 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<String> derive(Collection<FleetConfig.Profile> profiles) {
|
||||
Set<String> 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<String> 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<String> into, String name) {
|
||||
if (name != null && !name.isBlank()) {
|
||||
into.add(name);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>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.
|
||||
*
|
||||
* <p>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<String> allowed = MemberEnvAllowList.derive(List.of());
|
||||
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed);
|
||||
|
||||
Map<String, String> 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<String> baseline = exportedNamesFromCleanParent(cleanParent, null);
|
||||
Set<String> 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<String> 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<String> allowed = MemberEnvAllowList.derive(List.of());
|
||||
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed);
|
||||
|
||||
Map<String, String> 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<String> exportedNamesFromCleanParent(Map<String, String> 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<String> 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;
|
||||
}
|
||||
}
|
||||
@@ -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<String, String> 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<String> 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<String> 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<String> 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<String> before = MemberEnvAllowList.derive(List.of(first));
|
||||
|
||||
FleetConfig.Profile second = profile("second", "TOKEN_SECOND", "GIT_TOK", null,
|
||||
Map.of("SECOND_KEY", "v"));
|
||||
Set<String> 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<String> 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");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user