CB-633: SSH_AUTH_SOCK block wins over allow:, non-zsh fallback reports its gap #191
@@ -1000,8 +1000,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
*
|
||||
* <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}.
|
||||
* the ZDOTDIR scrub ({@link #applyEnvironmentAllowListPolicy}); {@code allow} adds explicit
|
||||
* keeps to its derived base, while {@code known} remains reporting metadata.
|
||||
*/
|
||||
private void applyMemberCredentialPolicy(Map<String, String> workerEnv) {
|
||||
FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get();
|
||||
@@ -1010,8 +1010,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
}
|
||||
if (!creds.isAllowList()) {
|
||||
overlayBlockedCredentials(workerEnv, creds);
|
||||
logCredentialGap(creds, null);
|
||||
}
|
||||
logCredentialGap(creds);
|
||||
}
|
||||
|
||||
/** Put {@link #BLOCKED_CREDENTIAL_SENTINEL} over every blocked name in the pane-creation env map. */
|
||||
@@ -1034,10 +1034,14 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
* 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.
|
||||
* MemberEnvAllowList#derive}) UNIONed with {@code memberCredentials.allow} and the exact keys of
|
||||
* THIS launch's env map. This lets the operator keep inherited variables by name without putting
|
||||
* their values in config, and names the daemon itself injects survive its own control. {@code
|
||||
* SSH_AUTH_SOCK} is EXCLUDED from that union and added ONLY when {@code
|
||||
* memberCredentials.sshAuthSock: allow} — {@code sshAuthSock} is the sole control for that one
|
||||
* name, so it cannot be widened back in by naming it on {@code allow:} too; a conflicting entry
|
||||
* there gets a WARN ({@link #warnSshAuthSockConflict}) and loses. By default {@code
|
||||
* SSH_AUTH_SOCK} 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();
|
||||
@@ -1053,17 +1057,31 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
// it), but strictly better than nothing.
|
||||
warnNonZsh(loginShell);
|
||||
overlayBlockedCredentials(launch.env(), creds);
|
||||
logCredentialGap(creds);
|
||||
// Nothing is scrubbed on this path — it is the CB-596 overlay only, which a sourced
|
||||
// startup file can undo. Report the gap with the WARN wording (an inherited-unblocked
|
||||
// exposure), not the allow-list INFO wording, which would falsely claim a scrub blanks it.
|
||||
logCredentialGap(creds, null);
|
||||
return null;
|
||||
}
|
||||
Set<String> allowed = new java.util.TreeSet<>(MemberEnvAllowList.derive(profiles.values()));
|
||||
for (String name : creds.allow()) {
|
||||
// SSH_AUTH_SOCK is governed ONLY by sshAuthSock: below, never by allow: — an operator
|
||||
// who writes both `sshAuthSock: block` and `SSH_AUTH_SOCK` on `allow:` means the block
|
||||
// to win, not to be silently widened away by the second control.
|
||||
if (!SSH_AUTH_SOCK.equals(name)) {
|
||||
allowed.add(name);
|
||||
}
|
||||
}
|
||||
if (creds.sshAuthSockAllowed()) {
|
||||
allowed.add(SSH_AUTH_SOCK);
|
||||
} else if (creds.allow().contains(SSH_AUTH_SOCK)) {
|
||||
warnSshAuthSockConflict();
|
||||
} // blocked by default: absent from the set ⇒ blanked by the scrub like any other name
|
||||
allowed.addAll(launch.env().keySet());
|
||||
logCredentialGap(creds, allowed);
|
||||
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 "
|
||||
log.info("memberCredentials policy=allow-list: profile={} generated ZDOTDIR {} — effective "
|
||||
+ "allow-list holds {} name(s); the pane reports allowed N of M at release",
|
||||
cfg.profile(), dir.getFileName(), allowed.size());
|
||||
return dir;
|
||||
@@ -1072,6 +1090,23 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
/** The operator ssh-agent handle — kept ONLY by explicit config decision, never by default. */
|
||||
private static final String SSH_AUTH_SOCK = "SSH_AUTH_SOCK";
|
||||
|
||||
/** Guards {@link #warnSshAuthSockConflict} to one WARN per launcher instance, not one per spawn. */
|
||||
private final AtomicBoolean sshAuthSockConflictWarned = new AtomicBoolean();
|
||||
|
||||
/**
|
||||
* Defect fix: {@code SSH_AUTH_SOCK} named on {@code memberCredentials.allow} while
|
||||
* {@code sshAuthSock} is not {@code allow} is a conflict between the two controls — say which
|
||||
* one wins, once per launcher instance, without printing any value.
|
||||
*/
|
||||
private void warnSshAuthSockConflict() {
|
||||
if (sshAuthSockConflictWarned.compareAndSet(false, true)) {
|
||||
log.warn("memberCredentials policy=allow-list: SSH_AUTH_SOCK is named on allow: but "
|
||||
+ "sshAuthSock is not 'allow' — sshAuthSock: block wins and SSH_AUTH_SOCK "
|
||||
+ "stays blocked. Remove it from allow: or set sshAuthSock: allow if the "
|
||||
+ "member should keep it.");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
@@ -1131,29 +1166,59 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
private static final Pattern CREDENTIAL_SHAPED_NAME =
|
||||
Pattern.compile("(?i).*(TOKEN|SECRET|_KEY|APIKEY|PASSWORD|CREDENTIAL|AUTH).*");
|
||||
|
||||
/** Guards {@link #logCredentialGap} to one WARN per launcher instance, not one per spawn. */
|
||||
private final AtomicBoolean credentialGapLogged = new AtomicBoolean();
|
||||
/**
|
||||
* Guards {@link #logCredentialGap}'s two report kinds separately — one flag per wording, not
|
||||
* one shared flag — so an early INFO ("will be blanked by the scrub") on one spawn can never
|
||||
* suppress a later, more serious WARN ("inherited UNBLOCKED") on another. {@code
|
||||
* memberCredentials} is live-reloadable, so the policy really can change between spawns on the
|
||||
* same launcher instance.
|
||||
*/
|
||||
private final AtomicBoolean allowListGapLogged = new AtomicBoolean();
|
||||
|
||||
/** See {@link #allowListGapLogged} — the WARN-wording counterpart. */
|
||||
private final AtomicBoolean unprotectedGapLogged = new AtomicBoolean();
|
||||
|
||||
/**
|
||||
* CB-596 criterion 4: a credential-shaped host env var name on neither {@code known} nor
|
||||
* {@code allow} is not silently allowed — it is reported. {@link #hostEnvNames} enumerates the
|
||||
* CB-596 criterion 4: report credential-shaped host env var names that the active policy does
|
||||
* not classify. Callers that pass a non-null {@code effectiveAllowed} are reporting against a
|
||||
* kept-name set a scrub will actually enforce — an absent name gets an INFO stating that the
|
||||
* scrub will blank it, because that is the control working rather than an exposure. Every
|
||||
* other caller — deny-by-default, and an allow-list spawn whose scrub could not run (non-zsh
|
||||
* login shell, falls back to the overlay) — passes {@code null} and gets the original WARN,
|
||||
* because in both cases a gap name is inherited unblocked. {@link #hostEnvNames} enumerates the
|
||||
* daemon's own environment (see that field's javadoc for why the daemon's env is read rather
|
||||
* than the spawned pane's, which the daemon has no channel to inspect at spawn time); this logs
|
||||
* every such NAME, at WARN, at most once per launcher instance — never a value, a prefix of a
|
||||
* value, or a hash of a value, so the log itself cannot leak anything.
|
||||
* than the spawned pane's). This logs names only, at most once per launcher instance per report
|
||||
* kind — never a value, a prefix of a value, or a hash of a value, so the log itself cannot
|
||||
* leak anything.
|
||||
*/
|
||||
private void logCredentialGap(FleetConfig.MemberCredentials creds) {
|
||||
Set<String> covered = new HashSet<>(creds.known());
|
||||
covered.addAll(creds.allow());
|
||||
private void logCredentialGap(FleetConfig.MemberCredentials creds, Set<String> effectiveAllowed) {
|
||||
Set<String> covered = effectiveAllowed == null
|
||||
? new HashSet<>(creds.known()) : effectiveAllowed;
|
||||
if (effectiveAllowed == null) {
|
||||
covered.addAll(creds.allow());
|
||||
}
|
||||
List<String> gap = hostEnvNames.get().stream()
|
||||
.filter(name -> CREDENTIAL_SHAPED_NAME.matcher(name).matches())
|
||||
.filter(name -> !covered.contains(name))
|
||||
.filter(name -> effectiveAllowed == null
|
||||
? !covered.contains(name) : !MemberEnvAllowList.keeps(covered, name))
|
||||
.sorted()
|
||||
.toList();
|
||||
if (gap.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
if (credentialGapLogged.compareAndSet(false, true)) {
|
||||
// effectiveAllowed != null means a derived allow-list set was actually computed (the
|
||||
// scrub will run against it) — that is the INFO case. A null effectiveAllowed means no
|
||||
// scrub protects this spawn (deny-by-default, or an allow-list spawn that fell back to
|
||||
// the overlay-only path because the login shell is not zsh) — that is the WARN case.
|
||||
// See #allowListGapLogged for why the two kinds use separate guards.
|
||||
if (effectiveAllowed != null) {
|
||||
if (allowListGapLogged.compareAndSet(false, true)) {
|
||||
log.info("memberCredentials allow-list: {} credential-shaped host env var name(s) "
|
||||
+ "are not kept and will be blanked by the scrub — {}. Add any name "
|
||||
+ "a member legitimately needs to memberCredentials.allow.",
|
||||
gap.size(), gap);
|
||||
}
|
||||
} else if (unprotectedGapLogged.compareAndSet(false, true)) {
|
||||
log.warn("memberCredentials gap: {} credential-shaped env var name(s) are on neither "
|
||||
+ "known: nor allow: — every member pane inherits them UNBLOCKED — {}. "
|
||||
+ "Add each to memberCredentials.known (blocked by default) or .allow "
|
||||
|
||||
@@ -7,14 +7,16 @@ 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.
|
||||
* CB-633: the base set of environment variable NAMES a spawned member is allowed to keep under
|
||||
* {@code memberCredentials.policy: allow-list}. This class derives the base from what the launcher
|
||||
* itself injects. The launcher then adds the operator's explicitly named {@code
|
||||
* memberCredentials.allow} keeps and the exact keys of this launch's env map.
|
||||
*
|
||||
* <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:
|
||||
* <p>A hand-typed list that <em>replaces</em> derivation is the defect this class exists to prevent:
|
||||
* a profile added to config later would silently break spawns whose scrub did not know its names.
|
||||
* An explicit list that only adds keeps is safe because adding a name can only widen the set; it
|
||||
* cannot make another spawn lose a name that derivation already kept. The derived base is the union
|
||||
* of:
|
||||
*
|
||||
* <ul>
|
||||
* <li>every configured {@link FleetConfig.Profile profile}'s {@code gitTokenEnv},
|
||||
@@ -26,11 +28,11 @@ import java.util.TreeSet;
|
||||
* 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>Because the base 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. The launcher then adds the operator's
|
||||
* explicit keeps and the exact keys of each spawn's own env map at generation time (see {@code
|
||||
* HerdrPeerLauncher}), so member binaries can keep named inherited variables without putting their
|
||||
* secret values in config, and anything the daemon 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
|
||||
|
||||
@@ -1025,7 +1025,7 @@ class ClaudeCodeLauncherTest {
|
||||
* {@code System.getenv()} so the test is deterministic.
|
||||
*/
|
||||
@Test
|
||||
void aCredentialShapedNameOnNeitherListIsLoggedAsAGap() {
|
||||
void denyByDefaultKeepsTheExactCredentialGapWarn() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = new FleetConfig.Profile(
|
||||
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
|
||||
@@ -1045,16 +1045,104 @@ class ClaudeCodeLauncherTest {
|
||||
logger.detachAppender(appender);
|
||||
}
|
||||
|
||||
String expected = "memberCredentials gap: 1 credential-shaped env var name(s) are on neither "
|
||||
+ "known: nor allow: — every member pane inherits them UNBLOCKED — "
|
||||
+ "[A_BRAND_NEW_SECRET_TOKEN]. Add each to memberCredentials.known (blocked by default) "
|
||||
+ "or .allow (if a member legitimately needs it).";
|
||||
assertTrue(appender.list.stream().anyMatch(e ->
|
||||
e.getFormattedMessage().contains("memberCredentials gap")
|
||||
&& e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN")),
|
||||
"the gap must name the unrecognized credential-shaped var, never a value");
|
||||
e.getLevel() == ch.qos.logback.classic.Level.WARN
|
||||
&& e.getFormattedMessage().equals(expected)),
|
||||
"deny-by-default must keep the exact existing WARN text");
|
||||
assertNotNull(startEnv(herdr).get("GITEA_ACCESS_TOKEN"),
|
||||
"deny-by-default must still overlay a known blocked credential");
|
||||
assertFalse(appender.list.stream().anyMatch(e -> e.getFormattedMessage().contains("PATH")),
|
||||
"PATH/HOME are not credential-shaped and must not be reported as a gap");
|
||||
assertFalse(appender.list.stream().anyMatch(e -> e.getFormattedMessage().contains("AI_GATEWAY_TOKEN")),
|
||||
"a name already on allow: is covered, not a gap");
|
||||
}
|
||||
|
||||
@Test
|
||||
void allowListReportsAnUnkeptCredentialAsBlankedWithoutSayingUnblocked() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = new FleetConfig.Profile(
|
||||
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
|
||||
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
|
||||
FleetConfig.MemberCredentials creds = new FleetConfig.MemberCredentials(
|
||||
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null);
|
||||
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
|
||||
name -> "SHELL".equals(name) ? "/bin/zsh" : null,
|
||||
0, System::currentTimeMillis, () -> {}, null, () -> creds,
|
||||
() -> Set.of("PATH", "HOME", "A_BRAND_NEW_SECRET_TOKEN"));
|
||||
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
|
||||
ch.qos.logback.classic.Level original = logger.getLevel();
|
||||
logger.setLevel(ch.qos.logback.classic.Level.INFO);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
svc.spawn();
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
logger.setLevel(original);
|
||||
}
|
||||
|
||||
ILoggingEvent gap = appender.list.stream()
|
||||
.filter(e -> e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN"))
|
||||
.findFirst().orElseThrow(() -> new AssertionError("allow-list must report the blanked name"));
|
||||
assertEquals(ch.qos.logback.classic.Level.INFO, gap.getLevel(),
|
||||
"a scrubbed credential name is the allow-list control working, not a WARN");
|
||||
assertTrue(gap.getFormattedMessage().contains("will be blanked by the scrub"));
|
||||
assertFalse(gap.getFormattedMessage().contains("UNBLOCKED"),
|
||||
"the allow-list report must not claim that a blanked name is exposed");
|
||||
}
|
||||
|
||||
/**
|
||||
* Defect fix: {@code memberCredentials} is live-reloadable, so the policy really can change
|
||||
* between spawns on the SAME launcher instance. An earlier INFO gap report (allow-list) must
|
||||
* not permanently suppress a later, more serious WARN gap report (deny-by-default) — the two
|
||||
* report kinds need separate guards, not one shared flag.
|
||||
*/
|
||||
@Test
|
||||
void anEarlierInfoGapDoesNotSuppressALaterWarnGapOnTheSameLauncher() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = new FleetConfig.Profile(
|
||||
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
|
||||
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
|
||||
AtomicReference<FleetConfig.MemberCredentials> creds = new AtomicReference<>(
|
||||
new FleetConfig.MemberCredentials(
|
||||
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null));
|
||||
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
|
||||
name -> "SHELL".equals(name) ? "/bin/zsh" : null,
|
||||
0, System::currentTimeMillis, () -> {}, null, creds::get,
|
||||
() -> Set.of("PATH", "HOME", "A_BRAND_NEW_SECRET_TOKEN"));
|
||||
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
|
||||
ch.qos.logback.classic.Level original = logger.getLevel();
|
||||
logger.setLevel(ch.qos.logback.classic.Level.INFO);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
svc.spawn(); // allow-list: logs the INFO gap report first
|
||||
creds.set(new FleetConfig.MemberCredentials(
|
||||
FleetConfig.MemberCredentials.POLICY_DENY_BY_DEFAULT, List.of(), List.of(), null));
|
||||
svc.spawn(); // deny-by-default: must still log its own WARN gap report
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
logger.setLevel(original);
|
||||
}
|
||||
|
||||
assertTrue(appender.list.stream().anyMatch(e ->
|
||||
e.getLevel() == ch.qos.logback.classic.Level.WARN
|
||||
&& e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN")
|
||||
&& e.getFormattedMessage().contains("UNBLOCKED")),
|
||||
"a prior INFO gap report must not suppress a later WARN gap report on the same "
|
||||
+ "launcher instance");
|
||||
}
|
||||
|
||||
/**
|
||||
* The half of CB-592 that can actually survive the pane's login shell. BRIDGED_MEMBER is a name
|
||||
* secrets.sh never exports, so nothing overwrites it — measured: GITEA_TOKEN is injected the
|
||||
|
||||
+160
-2
@@ -1,5 +1,8 @@
|
||||
package dev.ltms.fleet.member;
|
||||
|
||||
import ch.qos.logback.classic.Logger;
|
||||
import ch.qos.logback.classic.spi.ILoggingEvent;
|
||||
import ch.qos.logback.core.read.ListAppender;
|
||||
import dev.ltms.fleet.config.FleetConfig;
|
||||
import dev.ltms.fleet.herdr.AgentControl;
|
||||
import dev.ltms.fleet.herdr.FakeHerdr;
|
||||
@@ -8,7 +11,10 @@ import dev.ltms.fleet.peer.Capability;
|
||||
import dev.ltms.fleet.peer.MemberRole;
|
||||
import dev.ltms.fleet.peer.SpawnRequest;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.HashMap;
|
||||
@@ -21,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.junit.jupiter.api.Assumptions.assumeTrue;
|
||||
|
||||
/**
|
||||
* CB-633: proves the allow-list scrub is actually WIRED INTO the spawn path — not merely that its
|
||||
@@ -43,6 +50,8 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
|
||||
/** A name the daemon itself injects — it must survive its own scrub, so it must be allowed. */
|
||||
private static final String INJECTED = "ANTHROPIC_BASE_URL";
|
||||
private static final String OPERATOR_KEEP = "CONTEXT7_TOKEN";
|
||||
private static final String UNLISTED_CREDENTIAL = "A_BRAND_NEW_SECRET_TOKEN";
|
||||
|
||||
@Test
|
||||
void spawningUnderAllowListPolicyGivesThePaneAGeneratedZdotdir() {
|
||||
@@ -74,6 +83,43 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
+ "), or the daemon's own configuration is blanked by its own control");
|
||||
}
|
||||
|
||||
/** Explicit operator keeps widen the derived base, but all other exported names stay blocked. */
|
||||
@Test
|
||||
void operatorKeepSurvivesWhileAnUnlistedCredentialIsBlanked(@TempDir Path home) throws Exception {
|
||||
Path zsh = Path.of("/bin/zsh");
|
||||
assumeTrue(Files.isExecutable(zsh), "/bin/zsh not present — nothing to prove here");
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
WiringLauncher launcher = new WiringLauncher(herdr, allowList(List.of(OPERATOR_KEEP)));
|
||||
|
||||
var spawned = launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
|
||||
try {
|
||||
Path zdotdir = Path.of(launcher.env.get("ZDOTDIR"));
|
||||
ProcessBuilder probe = new ProcessBuilder(zsh.toString(), "-i", "-c",
|
||||
"[[ -n \"$" + OPERATOR_KEEP + "\" ]] && print -r -- kept || print -r -- missing; "
|
||||
+ "[[ -z \"$" + UNLISTED_CREDENTIAL
|
||||
+ "\" ]] && print -r -- blanked || print -r -- leaked");
|
||||
probe.environment().clear();
|
||||
probe.environment().putAll(Map.of(
|
||||
"HOME", home.toString(),
|
||||
"PATH", "/usr/bin:/bin",
|
||||
"SHELL", zsh.toString(),
|
||||
"ZDOTDIR", zdotdir.toString(),
|
||||
OPERATOR_KEEP, "needed-by-member-binary",
|
||||
UNLISTED_CREDENTIAL, "must-not-survive"));
|
||||
probe.redirectError(ProcessBuilder.Redirect.DISCARD);
|
||||
|
||||
Process process = probe.start();
|
||||
String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
|
||||
assertTrue(process.waitFor(60, java.util.concurrent.TimeUnit.SECONDS),
|
||||
"the zsh scrub probe did not exit within 60 seconds");
|
||||
assertEquals(0, process.exitValue(), "the zsh scrub probe must exit cleanly");
|
||||
assertEquals("kept\nblanked\n", output,
|
||||
"memberCredentials.allow must add a keep, while an unlisted credential stays blanked");
|
||||
} finally {
|
||||
launcher.stop(spawned.id());
|
||||
}
|
||||
}
|
||||
|
||||
/** The default policy must not generate anything — an upgrade changes nothing until asked. */
|
||||
@Test
|
||||
void spawningUnderTheDefaultPolicyGeneratesNoZdotdir() {
|
||||
@@ -103,9 +149,116 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
"bash ignores ZDOTDIR; setting it would be protection theatre");
|
||||
}
|
||||
|
||||
/**
|
||||
* Defect fix: under {@code policy: allow-list} with a non-zsh login shell, nothing is
|
||||
* scrubbed — the overlay-only fallback is the whole protection. The gap must still be
|
||||
* reported, and with the deny-by-default WARN wording ("inherited UNBLOCKED"), never the
|
||||
* allow-list INFO wording ("will be blanked by the scrub"), because nothing is scrubbed here.
|
||||
*/
|
||||
@Test
|
||||
void nonZshAllowListFallbackStillReportsTheGapAsAWarn() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/bash",
|
||||
() -> Set.of("PATH", "HOME", UNLISTED_CREDENTIAL));
|
||||
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
}
|
||||
|
||||
ILoggingEvent gap = appender.list.stream()
|
||||
.filter(e -> e.getFormattedMessage().contains(UNLISTED_CREDENTIAL))
|
||||
.findFirst().orElseThrow(() -> new AssertionError(
|
||||
"the non-zsh allow-list fallback must still report the credential gap"));
|
||||
assertEquals(ch.qos.logback.classic.Level.WARN, gap.getLevel(),
|
||||
"nothing is scrubbed on this path, so the report must use the WARN wording, not "
|
||||
+ "the allow-list INFO wording");
|
||||
assertTrue(gap.getFormattedMessage().contains("UNBLOCKED"),
|
||||
"the fallback path scrubs nothing, so the wording must say so");
|
||||
assertFalse(gap.getFormattedMessage().contains("will be blanked by the scrub"),
|
||||
"nothing is scrubbed on this path — that wording would be false here");
|
||||
}
|
||||
|
||||
/**
|
||||
* The live operator config has {@code SSH_AUTH_SOCK} on BOTH {@code allow:} AND
|
||||
* {@code sshAuthSock: block}. Defect fix: {@code sshAuthSock: block} must win — the allow:
|
||||
* entry must not silently widen the derived allow-list to include it.
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockBlockWinsOverAnAllowListEntry(@TempDir Path home) throws Exception {
|
||||
Path zsh = Path.of("/bin/zsh");
|
||||
assumeTrue(Files.isExecutable(zsh), "/bin/zsh not present");
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
Supplier<FleetConfig.MemberCredentials> live = () -> new FleetConfig.MemberCredentials(
|
||||
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST,
|
||||
List.of("AI_GATEWAY_TOKEN", "WORKER_GITEA_TOKEN", "CONTEXT7_TOKEN", "GITEA_HOST",
|
||||
"OPENCODE_AUTOMODE_MODEL", "SSH_AUTH_SOCK", "CLAUDE_CODE_MESSAGING_TOKEN"),
|
||||
List.of(), "block");
|
||||
WiringLauncher launcher = new WiringLauncher(herdr, live);
|
||||
var spawned = launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
|
||||
try {
|
||||
Path zdotdir = Path.of(launcher.env.get("ZDOTDIR"));
|
||||
ProcessBuilder probe = new ProcessBuilder(zsh.toString(), "-i", "-c",
|
||||
"[[ -n \"$SSH_AUTH_SOCK\" ]] && print -r -- LEAKED || print -r -- blanked");
|
||||
probe.environment().clear();
|
||||
probe.environment().putAll(Map.of(
|
||||
"HOME", home.toString(), "PATH", "/usr/bin:/bin", "SHELL", zsh.toString(),
|
||||
"ZDOTDIR", zdotdir.toString(), "SSH_AUTH_SOCK", "/tmp/agent.sock"));
|
||||
probe.redirectError(ProcessBuilder.Redirect.DISCARD);
|
||||
Process pr = probe.start();
|
||||
String out = new String(pr.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
|
||||
pr.waitFor(60, java.util.concurrent.TimeUnit.SECONDS);
|
||||
assertEquals("blanked\n", out, "sshAuthSock: block must win over allow:");
|
||||
} finally {
|
||||
launcher.stop(spawned.id());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Mirror of {@link #sshAuthSockBlockWinsOverAnAllowListEntry}: with {@code sshAuthSock: allow}
|
||||
* (no conflict with {@code allow:}), {@code SSH_AUTH_SOCK} IS kept. Without this case, a fix
|
||||
* that simply deletes {@code SSH_AUTH_SOCK} everywhere would also pass the block test.
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockAllowKeepsTheVariable(@TempDir Path home) throws Exception {
|
||||
Path zsh = Path.of("/bin/zsh");
|
||||
assumeTrue(Files.isExecutable(zsh), "/bin/zsh not present");
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
Supplier<FleetConfig.MemberCredentials> creds = () -> new FleetConfig.MemberCredentials(
|
||||
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST,
|
||||
List.of("SSH_AUTH_SOCK"), List.of(), "allow");
|
||||
WiringLauncher launcher = new WiringLauncher(herdr, creds);
|
||||
var spawned = launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
|
||||
try {
|
||||
Path zdotdir = Path.of(launcher.env.get("ZDOTDIR"));
|
||||
ProcessBuilder probe = new ProcessBuilder(zsh.toString(), "-i", "-c",
|
||||
"[[ -n \"$SSH_AUTH_SOCK\" ]] && print -r -- kept || print -r -- blanked");
|
||||
probe.environment().clear();
|
||||
probe.environment().putAll(Map.of(
|
||||
"HOME", home.toString(), "PATH", "/usr/bin:/bin", "SHELL", zsh.toString(),
|
||||
"ZDOTDIR", zdotdir.toString(), "SSH_AUTH_SOCK", "/tmp/agent.sock"));
|
||||
probe.redirectError(ProcessBuilder.Redirect.DISCARD);
|
||||
Process pr = probe.start();
|
||||
String out = new String(pr.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
|
||||
pr.waitFor(60, java.util.concurrent.TimeUnit.SECONDS);
|
||||
assertEquals("kept\n", out, "sshAuthSock: allow must keep SSH_AUTH_SOCK");
|
||||
} finally {
|
||||
launcher.stop(spawned.id());
|
||||
}
|
||||
}
|
||||
|
||||
private static Supplier<FleetConfig.MemberCredentials> allowList() {
|
||||
return allowList(List.of());
|
||||
}
|
||||
|
||||
private static Supplier<FleetConfig.MemberCredentials> allowList(List<String> allow) {
|
||||
return () -> new FleetConfig.MemberCredentials(
|
||||
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null);
|
||||
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, allow, List.of(), null);
|
||||
}
|
||||
|
||||
private static String readAll(Path p) {
|
||||
@@ -134,10 +287,15 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
}
|
||||
|
||||
WiringLauncher(FakeHerdr herdr, Supplier<FleetConfig.MemberCredentials> creds, String shell) {
|
||||
this(herdr, creds, shell, null);
|
||||
}
|
||||
|
||||
WiringLauncher(FakeHerdr herdr, Supplier<FleetConfig.MemberCredentials> creds, String shell,
|
||||
Supplier<Set<String>> hostEnvNames) {
|
||||
super("test", new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
Map.of("test", profile()), "test",
|
||||
name -> "SHELL".equals(name) ? shell : null,
|
||||
0, () -> 0L, () -> { }, null, creds);
|
||||
0, () -> 0L, () -> { }, null, creds, hostEnvNames);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
Reference in New Issue
Block a user