diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index ebf506c..2912a60 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -450,6 +450,74 @@ guard: - gx00.gw - gx01.gw +# Member credential policy (CB-596, gitea issue #82). A herdr pane runs a LOGIN shell, and that +# shell re-sources the operator's own secret store — so a spawned member inherits every credential +# the operator's shell holds, not just the ones bridged means to give it. Measured on this host: +# 31 credential names, all set, with only ONE (GITEA_ACCESS_TOKEN) blocked before this — and that +# block was a single name hardcoded in HerdrPeerLauncher.java, not driven by this file. This block +# replaces that hardcoded shadow. +# +# DENY-BY-DEFAULT, NOT A DENY-LIST. A deny-list (name the bad ones, let everything else through) is +# silently wrong the moment the operator's store gains a new secret — nothing would ever report it. +# Deny-by-default inverts that: `known` bounds the blast radius to names actually enumerated below, +# and EVERY one of them is blocked UNLESS it is also in `allow`. Omitting this block entirely (the +# shipped default) blocks NOTHING — unlike most optional blocks in this file, absence here is a real +# gap, not a safe "feature off". A name that is neither `known` nor `allow`-ed is not silently let +# 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 member process +# starts, shadowing whatever the login shell would otherwise put there. +# +# 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 +# allow: +# - AI_GATEWAY_TOKEN # named in a profile's tokenEnv (local/gx) — a member reaching the +# # gateway is by design, not a leak +# - WORKER_GITEA_TOKEN # the repo-scoped forge token a member needs to open its own PR (CB-302) +# - CONTEXT7_TOKEN # already decided as allowed by CB-593 +# - GITEA_HOST # not a credential — a hostname, paired with the forge token above +# known: +# - AI_GATEWAY_TOKEN +# - BESZEL_ADMIN_EMAIL +# - BESZEL_ADMIN_PASSWORD +# - BESZEL_HUB_URL +# - BESZEL_KEY +# - BESZEL_UNIVERSAL_TOKEN +# - BRAIN_MCP_TOKEN +# - CF_ACCOUNT_ID +# - CF_API_TOKEN +# - CF_USER_TOKEN +# - CONFLUENCE_API_TOKEN +# - CONFLUENCE_USERNAME +# - CONTEXT7_TOKEN +# - GITEA_ACCESS_TOKEN +# - GITEA_HOST +# - GITLAB_OAUTH_CLIENT_SECRET +# - GITLAB_PERSONAL_ACCESS_TOKEN +# - GRAFANA_ADMIN_PASSWORD +# - GRAFANA_ADMIN_USER +# - HASS_TOKEN +# - HW_PASSWORD +# - HW_USER +# - LTMS_API_KEY +# - MEMORY_MCP_TOKEN +# - METRICS_PUSH_TOKEN +# - OPENCODE_AUTOMODE_MODEL +# - TELEGRAM_BOT_TOKEN +# - TELEGRAM_CHAT_ID +# - TS_API_KEY +# - TS_AUTHKEY +# - WORKER_GITEA_TOKEN + # Spawn-readiness gate (CB-306). The launcher blocks until the worker's herdr status is # injectable (IDLE/BLOCKED/DONE) or the timeout elapses. 0 disables the gate. # NOTE: keys are camelCase — config is bound by plain Jackson with no naming strategy and diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 3543eb8..73d33d9 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -139,13 +139,15 @@ public final class Bridged { adapters.add(new ClaudeCodeLauncher(agents, spaces, guard, claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), - () -> config.get().fleet())); + () -> config.get().fleet(), + () -> config.get().memberCredentials())); } if (!opencodeProfiles.isEmpty()) { adapters.add(new OpenCodeLauncher(agents, spaces, opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), - () -> config.get().fleet())); + () -> config.get().fleet(), + () -> config.get().memberCredentials())); } AtomicReference> liveCountRef = new AtomicReference<>(_ -> 0); // CB-578 stage B: one quarantine tracker for the whole daemon, shared between the launcher diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index a6cac35..d838076 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -66,6 +66,9 @@ import java.util.Set; * {@code BackendQuarantine} built at startup, so it is DEFERRED: changing it * needs a restart, and a quarantine already running keeps whatever cooldown was * live when it started. + * @param memberCredentials deny-by-default policy (CB-596) for which of the operator's own host + * credentials a spawned member's pane inherits. {@code null} (the block + * omitted) blocks nothing — see {@link MemberCredentials}. */ @JsonIgnoreProperties(ignoreUnknown = true) public record BridgedConfig( @@ -85,7 +88,19 @@ public record BridgedConfig( String placement, Auth auth, ConfigReload configReload, - Integer quarantineCooldownSeconds) { + Integer quarantineCooldownSeconds, + MemberCredentials memberCredentials) { + + /** Back-compat form before the CB-596 {@code memberCredentials:} block was added. */ + public BridgedConfig(Bind bind, String herdrSocket, Map profiles, Guard guard, + String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs, + Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet, + LeadHeartbeat leadHeartbeat, Health health, String placement, Auth auth, + ConfigReload configReload, Integer quarantineCooldownSeconds) { + this(bind, herdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs, + spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth, + configReload, quarantineCooldownSeconds, null); + } /** Default cooldown (CB-578 stage B) when {@code quarantineCooldownSeconds} is absent/non-positive. */ public static final int DEFAULT_QUARANTINE_COOLDOWN_SECONDS = 1800; @@ -926,6 +941,58 @@ public record BridgedConfig( } } + /** + * CB-596: which of the operator's own host credentials a spawned member's pane may inherit. + * + *

A herdr pane runs a login shell that re-sources the operator's own secret store, so a + * member inherits every credential the operator's shell holds — measured at 31 names on this + * host, of which only one ({@code GITEA_ACCESS_TOKEN}) used to be blocked, and that block was a + * single name hardcoded in {@link HerdrPeerLauncher} rather than driven by config (gitea issue + * #82). This record replaces that hardcoded shadow with a config-driven one. + * + *

deny-by-default, not a deny-list. A deny-list (block these specific names, let + * everything else through) is silently wrong the moment a new secret is added to the operator's + * store — nothing would ever report it. Deny-by-default inverts that: {@link #known} bounds the + * blast radius to names the operator has actually enumerated, and every one of them is blocked + * 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. + * @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. + */ + @JsonIgnoreProperties(ignoreUnknown = true) + public record MemberCredentials(String policy, List allow, List known) { + + /** The only policy this build understands: block every {@code known} name not in {@code allow}. */ + public static final String POLICY_DENY_BY_DEFAULT = "deny-by-default"; + + public MemberCredentials { + policy = (policy == null || policy.isBlank()) ? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(); + allow = allow == null ? List.of() : List.copyOf(allow); + known = known == null ? List.of() : List.copyOf(known); + } + + /** {@link #allow} as a set, for membership checks. */ + public Set allowSet() { + return Set.copyOf(allow); + } + + /** {@link #known} minus {@link #allow} — the names a spawn must shadow. */ + public Set blockedSet() { + Set blocked = new java.util.LinkedHashSet<>(known); + blocked.removeAll(allowSet()); + return blocked; + } + } + /** * The candidate profiles an unqualified spawn of {@code role} chooses between, in definition * order (CB-557). @@ -976,7 +1043,8 @@ public record BridgedConfig( static final Set KNOWN_TOP_LEVEL_KEYS = Set.of( "bind", "herdrSocket", "profiles", "guard", "worktreeRoot", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet", - "leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds"); + "leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds", + "memberCredentials"); /** Load and validate config from {@code path}. */ public static BridgedConfig load(Path path) { @@ -990,6 +1058,7 @@ public record BridgedConfig( rejectUnknownKind(yaml); rejectUnknownAuthMode(yaml); rejectUnknownPlacement(yaml); + rejectUnknownMemberCredentialsPolicy(yaml); BridgedConfig cfg = YAML.readValue(yaml, BridgedConfig.class); // CB-606: validated here, eagerly, using PlacementPolicies.fromName as the single source // of truth — not lazily at first spawn (see CompositePeerLauncher's placementPolicy @@ -1418,6 +1487,45 @@ public record BridgedConfig( } } + /** The member-credential policies this build understands — {@link MemberCredentials#policy()}'s only valid value. */ + private static final Set KNOWN_MEMBER_CREDENTIALS_POLICIES = + Set.of(MemberCredentials.POLICY_DENY_BY_DEFAULT); + + /** + * Reject a {@code memberCredentials.policy} that is not {@link #KNOWN_MEMBER_CREDENTIALS_POLICIES} + * (CB-596), naming the value and the accepted set. + * + *

{@link MemberCredentials}'s compact constructor only lower-cases {@code policy} and defaults + * a blank one to {@link MemberCredentials#POLICY_DENY_BY_DEFAULT} — nothing rejects an actual + * typo like {@code deny-by-defualt}. There is only one policy today, so such a typo would + * currently behave identically to the real value by accident; the day a second policy exists + * that accident becomes a silent behavior change. Refuse it now, at config load, following the + * same pattern as {@link #rejectUnknownAuthMode} and {@link #rejectUnknownPlacement}. + * + * @param yaml the raw config text + * @throws IllegalStateException when {@code memberCredentials.policy} is a non-blank value not in + * {@link #KNOWN_MEMBER_CREDENTIALS_POLICIES} (case-insensitive) + */ + static void rejectUnknownMemberCredentialsPolicy(String yaml) { + Map raw; + try { + raw = YAML.readValue(yaml, Map.class); + } catch (IOException | IllegalArgumentException e) { + return; // a malformed file is reported by the real parse, not here + } + if (raw == null || !(raw.get("memberCredentials") instanceof Map mc)) { + return; + } + if (!(mc.get("policy") instanceof String policy) || policy.isBlank() + || KNOWN_MEMBER_CREDENTIALS_POLICIES.contains(policy.toLowerCase())) { + return; + } + throw new IllegalStateException("refusing to start: memberCredentials.policy=" + policy + + " is not recognized — accepted values are " + + String.join(", ", KNOWN_MEMBER_CREDENTIALS_POLICIES.stream().sorted().toList()) + + " (case-insensitive)."); + } + /** * Reject a top-level {@code placement:} policy name {@link PlacementPolicies#fromName} does not * recognize (CB-606), at config load rather than lazily at first spawn. @@ -1488,9 +1596,17 @@ public record BridgedConfig( // config that never mentions it should still get a sane cooldown rather than a null one. Integer quarantineCooldown = (quarantineCooldownSeconds != null && quarantineCooldownSeconds > 0) ? quarantineCooldownSeconds : DEFAULT_QUARANTINE_COOLDOWN_SECONDS; + // memberCredentials IS defaulted, like guard/lifecycle/auth above, so no reader ever sees a + // null. CB-596: an empty MemberCredentials (empty known, empty allow) blocks NOTHING — unlike + // guard/lifecycle, an absent block is not a safe "feature off" default here, it is a gap. It + // 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 bridged.example.yaml's memberCredentials: comment. + MemberCredentials mc = memberCredentials != null ? memberCredentials + : new MemberCredentials(null, List.of(), List.of()); return new BridgedConfig(b, herdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs, broker, primary, f, leadHeartbeat, health, placementOrDefault, a, configReload, - quarantineCooldown); + quarantineCooldown, mc); } /** diff --git a/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java index 3a9c42e..4bbab56 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java @@ -83,6 +83,21 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { fleet); } + /** + * Production constructor, plus the CB-596 {@code memberCredentials} policy supplier. + */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials) { + this(agents, spaces, guard, profiles, defaultProfile, env, + spawnReadyTimeoutMs, + System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), + fleet, memberCredentials); + } + /** * Full testability constructor. Every injectable collaborator is explicit so unit tests supply * fakes for the clock ({@code nowMillis}) and poll-loop wait ({@code sleeper}). The @@ -125,6 +140,39 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { this.guard = guard; } + /** + * Full testability constructor, plus the CB-596 {@code memberCredentials} policy supplier. + */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Supplier fleet, + Supplier memberCredentials) { + super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, + spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials); + this.guard = guard; + } + + /** + * Full testability constructor, plus an injectable host-env-names source for the CB-596 + * criterion-4 gap detector. Test seam only — every production call site leaves this at the + * default (the real {@code System.getenv()} key set) via the constructor above. + */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Supplier fleet, + Supplier memberCredentials, + Supplier> hostEnvNames) { + super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, + spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames); + this.guard = guard; + } + /** * {@inheritDoc} * diff --git a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java index ee3ab20..136cdc4 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java @@ -20,6 +20,7 @@ import org.slf4j.LoggerFactory; import java.security.SecureRandom; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -90,6 +91,27 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { */ private final Supplier fleet; + /** + * CB-596: the live {@code memberCredentials:} policy, read once per spawn (same hot-reload shape + * as {@link #fleet}). {@code null} — either the supplier itself, or what it returns — means no + * policy is configured and {@link #applyMemberCredentialPolicy} shadows nothing. + */ + private final Supplier memberCredentials; + + /** + * Enumerates the daemon's own process environment variable NAMES ONLY, never values — the CB-596 + * criterion-4 gap detector's data source (see {@link #logCredentialGap}). Injectable for tests; + * production always resolves to the real {@code System.getenv()} key set. + * + *

Deliberately the daemon's own environment, not the spawned pane's: nothing in the herdr + * client surface lets the daemon read back an arbitrary command's output from a pane before the + * peer starts in it, so there is no channel to inspect the pane's environment directly. The + * daemon's own process is started the same way (a login shell sourcing the same secret store — + * see CB-592's investigation of {@code secrets.sh}), so on a single-host deployment its env + * mirrors what the pane's login shell is about to export. + */ + private final Supplier> hostEnvNames; + /** The final instruction always requires a bridge reply when the bridge MCP is mounted. */ protected static final String REPLY_CHARTER = "You are a spawned member in the claude-bridge fleet. Every message you receive arrives " @@ -166,6 +188,41 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, Supplier fleet) { + this(namePrefix, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, + nowMillis, sleeper, fleet, null); + } + + /** + * As above, plus the live {@code memberCredentials} policy (CB-596). + * + * @param memberCredentials live member-credential policy, read once per spawn; {@code null} ⇒ + * no policy configured, so a spawn shadows nothing. A separate + * constructor rather than a new parameter on the one above, so every + * existing call site keeps the pre-CB-596 default without an edit. + */ + protected HerdrPeerLauncher(String namePrefix, AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Supplier fleet, + Supplier memberCredentials) { + this(namePrefix, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, + nowMillis, sleeper, fleet, memberCredentials, null); + } + + /** + * As above, plus an injectable {@link #hostEnvNames} source for the CB-596 gap detector. Test + * seam only — every production call site leaves this {@code null} and gets the real host env. + */ + protected HerdrPeerLauncher(String namePrefix, AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Supplier fleet, + Supplier memberCredentials, + Supplier> hostEnvNames) { this.fleet = fleet; this.namePrefix = namePrefix; this.agents = agents; @@ -176,6 +233,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { this.spawnReadyTimeoutMs = spawnReadyTimeoutMs; this.nowMillis = nowMillis; this.sleeper = sleeper; + this.memberCredentials = memberCredentials; + this.hostEnvNames = hostEnvNames != null ? hostEnvNames : () -> System.getenv().keySet(); } // --- adapter seams ------------------------------------------------------------------------- @@ -767,12 +826,13 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { } /** - * CB-592: overlay value that shadows the admin {@code GITEA_ACCESS_TOKEN} a herdr pane - * otherwise inherits from herdr's own login-shell process environment (gitea issue #77). - * herdr spawns a pane from its own process environment and layers our map on top — + * CB-596: overlay value that shadows any host credential a herdr pane otherwise inherits from + * herdr's own login-shell process environment (gitea issue #82, superseding CB-592's single + * hardcoded {@code GITEA_ACCESS_TOKEN} name — see {@link #applyMemberCredentialPolicy}). herdr + * spawns a pane from its own process environment and layers our map on top — * {@link dev.ltms.bridged.herdr.WorkspaceControl#createTab} and {@code #splitPane} send only * the keys we put in that map, so any key we never mention passes straight through from - * herdr's own shell, admin token included. + * herdr's own shell, admin credentials included. * *

Deliberately a non-blank sentinel, not {@code ""}. Whether an empty-string overlay value * overrides an inherited variable or is skipped as blank could not be settled by reading this @@ -782,21 +842,21 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * javadoc), and that is only demonstrated for a non-blank value, so this reuses the same, * proven-reliable shape rather than the unverified one. * - *

MEASURED ON A LIVE PANE, 2026-08-15: this sentinel alone does NOT hold. The overlay - * itself works — {@code GITEA_TOKEN} is injected here, is exported by no shell file, and does - * reach the pane. The sentinel loses one step later. A herdr pane runs a login shell, - * {@code ~/.zprofile} sources {@code ${SHARED_ENV}/tools/secrets.sh}, and that file does a plain - * unconditional {@code export GITEA_ACCESS_TOKEN=...}. A login shell overwrites a value already - * in the environment, so the real admin token is put back over this sentinel before the member - * process ever starts. That defeat applies to every name {@code secrets.sh} exports, - * and no launcher-side overlay can win against it. + *

MEASURED ON A LIVE PANE, 2026-08-15 (CB-592): this sentinel alone does NOT hold. The + * overlay itself works — {@code GITEA_TOKEN} is injected here, is exported by no shell file, and + * does reach the pane. The sentinel loses one step later. A herdr pane runs a login + * shell, {@code ~/.zprofile} sources {@code ${SHARED_ENV}/tools/secrets.sh}, and that file does a + * plain unconditional {@code export GITEA_ACCESS_TOKEN=...}. A login shell overwrites a value + * already in the environment, so the real admin token is put back over this sentinel before the + * member process ever starts. That defeat applies to every name {@code secrets.sh} + * exports, and no launcher-side overlay can win against it. * *

So this constant is not the control on its own — {@link #MEMBER_MARKER} is the other half. * Keeping the sentinel is still worth it: it is correct for any peer kind whose pane does not * start a login shell, and it makes the intent explicit at the one place every adapter passes. */ - private static final String BLOCKED_GITEA_ACCESS_TOKEN = - "blocked-by-bridged-cb592-see-gitea-issue-77"; + private static final String BLOCKED_CREDENTIAL_SENTINEL = + "blocked-by-bridged-cb596-see-gitea-issue-82"; /** * CB-592: marks a pane as a bridged member so a shell startup file can decline to export @@ -835,11 +895,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * dev.ltms.bridged.guard.SubscriptionGuard}, which is checked against the profile's * {@code baseUrl} and nothing else. * - *

The CB-592 shadow and marker are put in last, after the profile's own - * {@code env:}, so no profile — present or future — can restore the admin token, or hide that - * the pane is a member, by naming either in config. This is the one place both are applied: - * every {@code buildLaunch} in every adapter calls this first, so a new profile, and a peer - * kind not yet written, gets them for free. + *

The CB-596 credential shadow and the CB-592 marker are put in last, after the + * profile's own {@code env:}, so no profile — present or future — can restore a blocked + * credential, or hide that the pane is a member, by naming either in config. This is the one + * place both are applied: every {@code buildLaunch} in every adapter calls this first, so a new + * profile, and a peer kind not yet written, gets them for free. */ protected Map baseEnv(BridgedConfig.Profile cfg) { Map workerEnv = new LinkedHashMap<>(); @@ -850,11 +910,70 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { if (cfg != null && cfg.env() != null) { workerEnv.putAll(cfg.env()); } - workerEnv.put("GITEA_ACCESS_TOKEN", BLOCKED_GITEA_ACCESS_TOKEN); + applyMemberCredentialPolicy(workerEnv); workerEnv.put(MEMBER_MARKER, "1"); return workerEnv; } + /** + * CB-596: shadow every configured {@code memberCredentials.known} name that is not also + * {@code allow}-ed, replacing CB-592's single hardcoded {@code GITEA_ACCESS_TOKEN} name (gitea + * issue #82). An allow-listed name is deliberately left unmentioned here — see {@link + * #BLOCKED_CREDENTIAL_SENTINEL}'s javadoc for why an overlay entry is the only way to shadow an + * inherited value, which is exactly why an allowed name must get NO entry: any entry at all, + * blank or not, risks overriding the real value the pane needs. + * + *

No {@code memberCredentials} configured — the supplier is {@code null}, or it resolves to + * one whose {@code known} list is empty — shadows nothing. This is a real, config-driven gap + * (see {@link BridgedConfig.MemberCredentials}'s javadoc), not a safe default: deny-by-default + * only defends names the operator has actually enumerated in {@code known}. + */ + private void applyMemberCredentialPolicy(Map workerEnv) { + BridgedConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get(); + if (creds == null) { + return; + } + for (String name : creds.blockedSet()) { + workerEnv.put(name, BLOCKED_CREDENTIAL_SENTINEL); + } + logCredentialGap(creds); + } + + /** Credential-shaped env var name heuristic for {@link #logCredentialGap} — case-insensitive. */ + 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(); + + /** + * 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 + * 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. + */ + private void logCredentialGap(BridgedConfig.MemberCredentials creds) { + Set covered = new HashSet<>(creds.known()); + covered.addAll(creds.allow()); + List gap = hostEnvNames.get().stream() + .filter(name -> CREDENTIAL_SHAPED_NAME.matcher(name).matches()) + .filter(name -> !covered.contains(name)) + .sorted() + .toList(); + if (gap.isEmpty()) { + return; + } + if (credentialGapLogged.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 " + + "(if a member legitimately needs it).", + gap.size(), gap); + } + } + /** Defensive copy of {@code argv} plus room to append launch flags. */ protected static List mutableArgv(List argv) { return new ArrayList<>(argv); diff --git a/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java index 8eb300e..e608a5f 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java @@ -104,6 +104,20 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { defaultConfigRoot(), defaultDiscoveryRoot(), fleet); } + /** + * Production constructor, plus the CB-596 {@code memberCredentials} policy supplier. + */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, long spawnReadyPollMs, + Supplier fleet, + Supplier memberCredentials) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, + System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), + defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials); + } + /** * Full testability constructor. Every injectable collaborator is explicit so unit tests supply a * fake clock ({@code nowMillis}), poll-loop wait ({@code sleeper}), and a temp {@code configRoot} @@ -151,6 +165,23 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { this.discovery = new OpenCodeSessionDiscovery(discoveryRoot); } + /** + * Full testability constructor, plus the CB-596 {@code memberCredentials} policy supplier. + */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Path configRoot, Path discoveryRoot, + Supplier fleet, + Supplier memberCredentials) { + super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, + spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials); + this.configRoot = configRoot; + this.discovery = new OpenCodeSessionDiscovery(discoveryRoot); + } + private static Path defaultConfigRoot() { return Path.of(System.getProperty("java.io.tmpdir")); } diff --git a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java index 5ccb2d9..30cb531 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -1196,6 +1196,10 @@ class BridgedConfigTest { idleAfterSeconds: 600 backoffMs: 45000 quietNudgeCap: 5 + memberCredentials: + policy: deny-by-default + allow: [AI_GATEWAY_TOKEN] + known: [AI_GATEWAY_TOKEN, GITEA_ACCESS_TOKEN] """); BridgedConfig cfg = BridgedConfig.load(f); @@ -1226,6 +1230,11 @@ class BridgedConfigTest { assertEquals(600, cfg.leadHeartbeat().idleAfterSeconds(), "leadHeartbeat binds at the top level"); assertEquals(45_000L, cfg.leadHeartbeat().backoffMs()); assertEquals(5, cfg.leadHeartbeat().quietNudgeCap()); + + assertEquals(BridgedConfig.MemberCredentials.POLICY_DENY_BY_DEFAULT, cfg.memberCredentials().policy(), + "memberCredentials binds at the top level"); + assertEquals(Set.of("AI_GATEWAY_TOKEN"), cfg.memberCredentials().allowSet()); + assertEquals(Set.of("GITEA_ACCESS_TOKEN"), cfg.memberCredentials().blockedSet()); } /** @@ -1454,6 +1463,75 @@ class BridgedConfigTest { "error names the accepted set: " + e.getMessage()); } + /** + * CB-596: {@code memberCredentials.policy} is validated the same way {@code auth.mode} and + * per-profile {@code placement} are (CB-606's pattern) — a typo must not silently behave as the + * one real policy, because the day a second policy exists that silent fallback becomes a real + * behavior change instead of a happy accident. + */ + @Test + void unknownMemberCredentialsPolicyIsRefusedAtLoadNamingTheValueAndTheAcceptedSet(@TempDir Path dir) throws Exception { + Path f = dir.resolve("member-credentials-policy-typo.yaml"); + Files.writeString(f, """ + bind: + host: 127.0.0.1 + port: 8765 + memberCredentials: + policy: deny-by-defualt + known: + - GITEA_ACCESS_TOKEN + """); + + IllegalStateException e = assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("deny-by-defualt"), "error names the bad value: " + e.getMessage()); + assertTrue(e.getMessage().contains("deny-by-default"), "error names the accepted set: " + e.getMessage()); + } + + /** {@code allow}/{@code known} bind and {@link BridgedConfig.MemberCredentials#blockedSet()} is known minus allow. */ + @Test + void memberCredentialsBindsAllowAndKnownAndComputesBlockedSet(@TempDir Path dir) throws Exception { + Path f = dir.resolve("member-credentials.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + memberCredentials: + policy: deny-by-default + allow: + - AI_GATEWAY_TOKEN + - WORKER_GITEA_TOKEN + known: + - AI_GATEWAY_TOKEN + - WORKER_GITEA_TOKEN + - GITEA_ACCESS_TOKEN + - GITLAB_PERSONAL_ACCESS_TOKEN + """); + + BridgedConfig.MemberCredentials mc = BridgedConfig.load(f).memberCredentials(); + assertEquals(BridgedConfig.MemberCredentials.POLICY_DENY_BY_DEFAULT, mc.policy()); + assertEquals(Set.of("AI_GATEWAY_TOKEN", "WORKER_GITEA_TOKEN"), mc.allowSet()); + assertEquals(Set.of("GITEA_ACCESS_TOKEN", "GITLAB_PERSONAL_ACCESS_TOKEN"), mc.blockedSet(), + "blockedSet is known minus allow"); + } + + /** + * 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 + * gets — see {@link BridgedConfig#withDefaults()}), but it also must not pretend anything is + * blocked: an empty {@code known} list blocks nothing, and that is a real gap the operator must + * close by configuring this block, not a safe default. + */ + @Test + void absentMemberCredentialsDefaultsToAnEmptyNonNullBlock(@TempDir Path dir) throws Exception { + Path f = dir.resolve("member-credentials-absent.yaml"); + Files.writeString(f, "bind:\n port: 8080\n"); + + BridgedConfig.MemberCredentials mc = BridgedConfig.load(f).memberCredentials(); + assertNotNull(mc, "withDefaults() must never leave this null"); + assertTrue(mc.known().isEmpty(), "no known list configured — nothing is blocked"); + assertTrue(mc.allowSet().isEmpty()); + assertTrue(mc.blockedSet().isEmpty()); + } + @Test void absentProfilePlacementDefaultsToTab(@TempDir Path dir) throws Exception { Path f = dir.resolve("placement-absent.yaml"); diff --git a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java index b2f015e..068ca06 100644 --- a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java @@ -1,5 +1,8 @@ package dev.ltms.bridged.member; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import dev.ltms.bridged.config.BridgedConfig; import dev.ltms.bridged.guard.GuardException; import dev.ltms.bridged.guard.SubscriptionGuard; @@ -12,6 +15,7 @@ import dev.ltms.bridged.peer.PeerHandle; import dev.ltms.bridged.peer.PeerUnreachableException; import dev.ltms.bridged.peer.SpawnRequest; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import java.util.List; import java.util.Map; @@ -676,39 +680,90 @@ class ClaudeCodeLauncherTest { "the guard-checked baseUrl must win over any env: entry, or the boundary is bypassable"); } - // --- CB-592: the admin GITEA_ACCESS_TOKEN never reaches a member ----------------------------- + // --- CB-596: config-driven member-credential policy (replaces CB-592's hardcoded single name) -- - /** - * herdr's env map is an overlay onto its own (login-shell) process environment, so a worker - * inherits whatever the daemon's shell carries — including the admin GITEA_ACCESS_TOKEN — for - * every key baseEnv does not explicitly shadow. This pins that the launcher DOES send an - * explicit (non-blank) GITEA_ACCESS_TOKEN to herdr on every spawn, whatever the profile is, so - * a future baseEnv refactor cannot silently drop it and reopen the leak. Asserted against what - * tab.create's params actually carry, not an internal map built in the test (gitea #77). - * - *

Scope, measured on a live pane 2026-08-15: this pins what the launcher SENDS, and that is - * all it can pin. It does not prove the value survives, and it does not: the pane runs a login - * shell, ~/.zprofile sources secrets.sh, and its unconditional `export GITEA_ACCESS_TOKEN=...` - * puts the real token back over this sentinel. Closing that needs the operator to guard the - * export on BRIDGED_MEMBER — see everySpawnMarksThePaneAsAMember below. - */ - @Test - void everySpawnShadowsTheAdminGiteaAccessToken() { - FakeHerdr herdr = new FakeHerdr(); - service(herdr, List.of("claude"), null).spawn(); + /** A representative {@code memberCredentials} — 4 allowed, 4 blocked, matching the real ticket shape. */ + private static final BridgedConfig.MemberCredentials TEST_MEMBER_CREDENTIALS = new BridgedConfig.MemberCredentials( + null, + List.of("AI_GATEWAY_TOKEN", "WORKER_GITEA_TOKEN", "CONTEXT7_TOKEN", "GITEA_HOST"), + List.of("AI_GATEWAY_TOKEN", "WORKER_GITEA_TOKEN", "CONTEXT7_TOKEN", "GITEA_HOST", + "GITEA_ACCESS_TOKEN", "GITLAB_PERSONAL_ACCESS_TOKEN", "TS_AUTHKEY", "HASS_TOKEN")); - String shadowed = startEnv(herdr).get("GITEA_ACCESS_TOKEN"); - assertNotNull(shadowed, "GITEA_ACCESS_TOKEN must be explicitly overlaid, not left unmentioned"); - assertFalse(shadowed.isBlank(), "a blank overlay value's override behaviour is unverified — must be non-blank"); + private ClaudeCodeLauncher serviceWithCredentials(FakeHerdr herdr, BridgedConfig.MemberCredentials creds) { + BridgedConfig.Profile cfg = new BridgedConfig.Profile( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "worker: {profile} #{n}", null, null, null); + return new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + _ -> null, 0, System::currentTimeMillis, () -> {}, null, () -> creds); } /** - * No profile — present or future — may restore the admin token by naming it in {@code env:}. + * herdr's env map is an overlay onto its own (login-shell) process environment, so a worker + * inherits whatever the daemon's shell carries — including the operator's own credentials — for + * every key {@code baseEnv} does not explicitly shadow. This pins that every {@code known} name + * NOT also {@code allow}-ed gets an explicit (non-blank) sentinel overlay, whatever the profile + * is. Asserted against what tab.create's params actually carry, not an internal map built in the + * test (gitea #82). + * + *

Scope, measured on a live pane 2026-08-15 (CB-592): this pins what the launcher SENDS, and + * that is all a unit test can pin. It does not prove the value survives, and for names the + * operator's secrets.sh also exports it does not: the pane runs a login shell that puts the real + * value back over this sentinel unless the export is guarded on BRIDGED_MEMBER — see + * everySpawnMarksThePaneAsAMember below. + */ + @Test + void everyKnownNameNotAllowedIsShadowedWithTheSentinel() { + FakeHerdr herdr = new FakeHerdr(); + serviceWithCredentials(herdr, TEST_MEMBER_CREDENTIALS).spawn(); + + Map env = startEnv(herdr); + for (String blocked : List.of("GITEA_ACCESS_TOKEN", "GITLAB_PERSONAL_ACCESS_TOKEN", "TS_AUTHKEY", "HASS_TOKEN")) { + String shadowed = env.get(blocked); + assertNotNull(shadowed, blocked + " must be explicitly overlaid, not left unmentioned"); + assertFalse(shadowed.isBlank(), blocked + "'s overlay value must be non-blank"); + } + } + + /** + * An {@code allow}-ed name must get NO overlay entry at all — any entry, blank or not, risks + * overriding the real value the pane needs, and the whole point of {@code allow} is that the + * pane's own inherited value passes through untouched. + */ + @Test + void everyAllowedNameGetsNoOverlayEntrySoTheRealValuePassesThrough() { + FakeHerdr herdr = new FakeHerdr(); + serviceWithCredentials(herdr, TEST_MEMBER_CREDENTIALS).spawn(); + + Map env = startEnv(herdr); + for (String allowed : List.of("AI_GATEWAY_TOKEN", "WORKER_GITEA_TOKEN", "CONTEXT7_TOKEN", "GITEA_HOST")) { + assertFalse(env.containsKey(allowed), + allowed + " is allow-listed — the launcher must not mention it at all"); + } + } + + /** + * No {@code memberCredentials} configured (the pre-CB-596 constructor overloads still used + * throughout this file, and the shape a fresh {@code bridged.yaml} with no memberCredentials: + * block resolves to) blocks NOTHING. This documents the transitional gap rather than hiding it — + * see {@code HerdrPeerLauncher#applyMemberCredentialPolicy}'s javadoc. + */ + @Test + void noMemberCredentialsConfiguredBlocksNothing() { + FakeHerdr herdr = new FakeHerdr(); + service(herdr, List.of("claude"), null).spawn(); + + assertNull(startEnv(herdr).get("GITEA_ACCESS_TOKEN"), + "with no memberCredentials configured, nothing is shadowed — config must supply the policy"); + } + + /** + * No profile — present or future — may restore a blocked name by naming it in {@code env:}. * The shadow is applied after the profile's own env in {@link HerdrPeerLauncher#baseEnv} * precisely so this can never happen; this test pins that ordering. */ @Test - void aProfileEnvEntryCannotRestoreTheAdminGiteaAccessToken() { + void aProfileEnvEntryCannotRestoreABlockedName() { FakeHerdr herdr = new FakeHerdr(); BridgedConfig.Profile cfg = new BridgedConfig.Profile( "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", @@ -716,10 +771,48 @@ class ClaudeCodeLauncherTest { null, Map.of("GITEA_ACCESS_TOKEN", "admin-secret-from-profile-config"), null, null); new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), - _ -> null).spawn(); + _ -> null, 0, System::currentTimeMillis, () -> {}, null, () -> TEST_MEMBER_CREDENTIALS) + .spawn(cfg.profile(), null, null); assertNotEquals("admin-secret-from-profile-config", startEnv(herdr).get("GITEA_ACCESS_TOKEN"), - "a profile's own env: must not be able to smuggle the admin token back in"); + "a profile's own env: must not be able to smuggle a blocked name back in"); + } + + /** + * CB-596 criterion 4: a credential-shaped host env var name on neither {@code known} nor + * {@code allow} is not silently allowed — it must be reported (never its value). This pins the + * WARN naming the gap, using an injected host-env-names source rather than the real + * {@code System.getenv()} so the test is deterministic. + */ + @Test + void aCredentialShapedNameOnNeitherListIsLoggedAsAGap() { + FakeHerdr herdr = new FakeHerdr(); + BridgedConfig.Profile cfg = new BridgedConfig.Profile( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "worker: {profile} #{n}", null, null, null); + ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + _ -> null, 0, System::currentTimeMillis, () -> {}, null, () -> TEST_MEMBER_CREDENTIALS, + () -> Set.of("PATH", "HOME", "AI_GATEWAY_TOKEN", "A_BRAND_NEW_SECRET_TOKEN")); + + Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + try { + svc.spawn(); + } finally { + logger.detachAppender(appender); + } + + 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"); + 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"); } /** diff --git a/bridged/src/test/java/dev/ltms/bridged/member/OpenCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/member/OpenCodeLauncherTest.java index 0ee1bfa..a7b5010 100644 --- a/bridged/src/test/java/dev/ltms/bridged/member/OpenCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/member/OpenCodeLauncherTest.java @@ -52,6 +52,14 @@ class OpenCodeLauncherTest { 0, System::currentTimeMillis, () -> { }, configRoot, configRoot, fleet); } + private static OpenCodeLauncher serviceWithCredentials(FakeHerdr herdr, Path configRoot, + BridgedConfig.Profile cfg, + BridgedConfig.MemberCredentials creds) { + return new OpenCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + Map.of(cfg.profile(), cfg), cfg.profile(), k -> "GITEA_ACCESS_TOKEN".equals(k) ? "tok" : null, + 0, System::currentTimeMillis, () -> { }, configRoot, configRoot, null, () -> creds); + } + @SuppressWarnings("unchecked") private static Map lastStart(FakeHerdr herdr) { return (Map) herdr.lastCall("agent.start").params(); @@ -209,18 +217,33 @@ class OpenCodeLauncherTest { } /** - * CB-592: the shadow lives in {@link HerdrPeerLauncher#baseEnv}, shared by every adapter — this - * pins that the opencode path gets it too, not just Claude's. See the matching test in - * {@code ClaudeCodeLauncherTest} for the full rationale (gitea #77). + * CB-596: the config-driven shadow lives in {@link HerdrPeerLauncher#baseEnv}, shared by every + * adapter — this pins that the opencode path gets it too, not just Claude's. See the matching + * tests in {@code ClaudeCodeLauncherTest} for the full rationale (gitea #82, superseding CB-592's + * single hardcoded name). */ @Test - void everySpawnShadowsTheAdminGiteaAccessToken(@TempDir Path root) { + void aKnownNameNotAllowedIsShadowedWithTheSentinel(@TempDir Path root) { FakeHerdr herdr = new FakeHerdr(); - service(herdr, root, opencodeCfg(null, null, null)).spawn(); + BridgedConfig.MemberCredentials creds = new BridgedConfig.MemberCredentials( + null, List.of("AI_GATEWAY_TOKEN"), List.of("AI_GATEWAY_TOKEN", "GITEA_ACCESS_TOKEN")); + serviceWithCredentials(herdr, root, opencodeCfg(null, null, null), creds).spawn(); String shadowed = startEnv(herdr).get("GITEA_ACCESS_TOKEN"); assertNotNull(shadowed, "GITEA_ACCESS_TOKEN must be explicitly overlaid, not left unmentioned"); assertFalse(shadowed.isBlank(), "a blank overlay value's override behaviour is unverified — must be non-blank"); + assertFalse(startEnv(herdr).containsKey("AI_GATEWAY_TOKEN"), + "an allow-listed name must get no overlay entry at all"); + } + + /** No {@code memberCredentials} configured (the pre-CB-596 constructor overloads) blocks nothing. */ + @Test + void noMemberCredentialsConfiguredBlocksNothing(@TempDir Path root) { + FakeHerdr herdr = new FakeHerdr(); + service(herdr, root, opencodeCfg(null, null, null)).spawn(); + + assertNull(startEnv(herdr).get("GITEA_ACCESS_TOKEN"), + "with no memberCredentials configured, nothing is shadowed — config must supply the policy"); } @Test