From d42c2bc2047bcc26916636c58e3fedae1e581bc7 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 3 Sep 2026 20:12:46 +0700 Subject: [PATCH] fleetd #266: rename SSH agent environment setting --- fleetd/fleetd.example.yaml | 8 +- .../dev/ltms/fleet/config/FleetConfig.java | 35 ++++-- .../ltms/fleet/member/HerdrPeerLauncher.java | 4 +- .../ltms/fleet/member/MemberEnvAllowList.java | 6 +- .../ltms/fleet/config/FleetConfigTest.java | 101 +++++++++++++++--- .../HerdrPeerLauncherAllowListWiringTest.java | 6 +- .../fleet/member/MemberEnvAllowListTest.java | 4 +- 7 files changed, 126 insertions(+), 38 deletions(-) diff --git a/fleetd/fleetd.example.yaml b/fleetd/fleetd.example.yaml index ae44804..5788a06 100644 --- a/fleetd/fleetd.example.yaml +++ b/fleetd/fleetd.example.yaml @@ -676,8 +676,8 @@ guard: # 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 is omitted -# from the member environment ("block", the default). Blocking it only omits the +# sshAgentEnv → whether SSH_AUTH_SOCK may pass through under allow-list ("inherit") or is omitted +# from the member environment ("omit", the default). Omitting it only omits the # inherited ssh-agent path. It discourages automatic use of the operator's agent. # It does not deny same-user access to that socket. It also does not block SSH keys that # are readable on disk. Git over SSH may still work from inside a member. Keep the block: @@ -686,7 +686,7 @@ guard: # confidentiality boundary. A real boundary needs a different OS user or OS-level # confinement, such as a container or VM. That is the open question in fleetd #184. # -# Still do not set this to "allow" casually. SSH_AUTH_SOCK is a live handle to YOUR +# Still do not set this to "inherit" casually. SSH_AUTH_SOCK is a live handle to YOUR # ssh-agent, so a member holding it can sign with EVERY key the agent holds. It sits in # no secret file and looks like no credential, which is why it slipped past three # earlier tickets (gitea #110). Blocking it does not contain a member, but allowing it @@ -704,7 +704,7 @@ guard: # are unaffected either way. # memberCredentials: # policy: deny-by-default # or "deny-list", or "allow-list" (CB-633) — see above -# sshAuthSock: block # allow-list only; see the sshAuthSock note above +# sshAgentEnv: omit # allow-list only; see the sshAgentEnv note above # allow: # - AI_GATEWAY_TOKEN # named in a profile's tokenEnv (local/gx) — a member reaching the # # gateway is by design, not a leak diff --git a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java index d30f365..c759f41 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java +++ b/fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java @@ -1,6 +1,8 @@ package dev.ltms.fleet.config; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.ObjectMapper; @@ -1357,19 +1359,19 @@ public record FleetConfig( * 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). + * @param sshAgentEnv whether the member may inherit {@code SSH_AUTH_SOCK} under the allow-list + * policy ({@code "inherit"}) or must have it omitted ({@code "omit"}, 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 + * Omitting it does not prevent git over SSH inside the member; inherit it only when members do * not need to authenticate as the operator over SSH. Ignored under deny-list / * deny-by-default, which never touch the name. */ @JsonIgnoreProperties(ignoreUnknown = true) public record MemberCredentials(String policy, List allow, List known, - String sshAuthSock) { + String sshAgentEnv) { /** 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"; @@ -1386,11 +1388,25 @@ public record FleetConfig( */ public static final String POLICY_ALLOW_LIST = "allow-list"; - /** The pre-CB-633 three-field form — {@code sshAuthSock} defaults to blocked. */ + /** The pre-CB-633 three-field form — {@code sshAgentEnv} defaults to omitted. */ public MemberCredentials(String policy, List allow, List known) { this(policy, allow, known, null); } + /** + * Reads both the current {@code sshAgentEnv} key and the compatible {@code sshAuthSock} key. + * When both keys are present, {@code sshAgentEnv} wins, even if its value is unrecognised. + */ + @JsonCreator + public static MemberCredentials fromYaml(@JsonProperty("policy") String policy, + @JsonProperty("allow") List allow, + @JsonProperty("known") List known, + @JsonProperty("sshAgentEnv") String sshAgentEnv, + @JsonProperty("sshAuthSock") String sshAuthSock) { + return new MemberCredentials(policy, allow, known, + sshAgentEnv != null ? sshAgentEnv : sshAuthSock); + } + public MemberCredentials { String normalizedPolicy = (policy == null || policy.isBlank()) ? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(java.util.Locale.ROOT); @@ -1399,8 +1415,9 @@ public record FleetConfig( 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"; + sshAgentEnv = (sshAgentEnv != null + && ("inherit".equalsIgnoreCase(sshAgentEnv.trim()) + || "allow".equalsIgnoreCase(sshAgentEnv.trim()))) ? "inherit" : "omit"; } /** True when this block selects the CB-633 derived-allow-list policy. */ @@ -1409,8 +1426,8 @@ public record FleetConfig( } /** True when {@code SSH_AUTH_SOCK} may pass through under the allow-list policy. Default: no. */ - public boolean sshAuthSockAllowed() { - return "allow".equals(sshAuthSock); + public boolean sshAgentEnvInherited() { + return "inherit".equals(sshAgentEnv); } /** {@link #allow} as a set, for membership checks. */ diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java index 990b076..5fcef01 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -1471,9 +1471,9 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { Set brokerUriEnvNames = brokerUriEnvNames(); Set allowed = new java.util.TreeSet<>( MemberEnvAllowList.derive(profiles.values(), creds.allowSet(), brokerUriEnvNames)); - if (creds.sshAuthSockAllowed()) { + if (creds.sshAgentEnvInherited()) { allowed.add(SSH_AUTH_SOCK); - } // blocked by default: absent from the set ⇒ blanked by the scrub like any other name + } // omitted by default: absent from the set ⇒ blanked by the scrub like any other name allowed.addAll(launch.env().keySet()); allowed.removeAll(brokerUriEnvNames); return allowed; diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java b/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java index 07d927b..f1a7e67 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/MemberEnvAllowList.java @@ -34,7 +34,7 @@ import java.util.TreeSet; * *

{@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. + * ({@code memberCredentials.sshAgentEnv: inherit}), not a derivation default. * *

CB-633 follow-up: the union also includes {@code memberCredentials.allow:} — the * operator's own explicit list. Before this, {@code policy: allow-list} silently ignored every name @@ -42,7 +42,7 @@ import java.util.TreeSet; * turning the policy on could blank credentials working members already depended on. {@code * SSH_AUTH_SOCK} and configured broker URI environment names are exceptions: even when the operator * lists them under {@code allow:}, they are excluded here. {@code SSH_AUTH_SOCK} is added back ONLY - * by the caller when {@code sshAuthSock: allow} is explicitly set + * by the caller when {@code sshAgentEnv: inherit} is explicitly set * (see {@link #SSH_AUTH_SOCK}'s javadoc) — it is a live handle to the operator's own ssh-agent, not * a value, so treating it like any other allow-listed name would hand a member every key the * operator's agent holds the moment they typed the name under {@code allow:} for an unrelated @@ -53,7 +53,7 @@ public final class MemberEnvAllowList { /** * The operator's ssh-agent socket path. Deliberately excluded from {@link #derive}'s union of * {@code memberCredentials.allow:} — see the class javadoc's CB-633 follow-up note. Governed - * ONLY by {@code memberCredentials.sshAuthSock}, never by appearing in {@code allow:}. + * ONLY by {@code memberCredentials.sshAgentEnv}, never by appearing in {@code allow:}. */ public static final String SSH_AUTH_SOCK = "SSH_AUTH_SOCK"; diff --git a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java index bf07edf..3dcab3a 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigTest.java @@ -2010,22 +2010,93 @@ class FleetConfigTest { "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"); + void sshAgentEnvAcceptsEveryCompatibleKeyAndValuePair(@TempDir Path dir) throws Exception { + String[][] spellings = { + {"sshAuthSock", "block", "omit"}, + {"sshAuthSock", "allow", "inherit"}, + {"sshAuthSock", "omit", "omit"}, + {"sshAuthSock", "inherit", "inherit"}, + {"sshAgentEnv", "block", "omit"}, + {"sshAgentEnv", "allow", "inherit"}, + {"sshAgentEnv", "omit", "omit"}, + {"sshAgentEnv", "inherit", "inherit"} + }; + + for (int i = 0; i < spellings.length; i++) { + Path file = dir.resolve("member-credentials-ssh-agent-" + i + ".yaml"); + Files.writeString(file, """ + bind: + port: 8080 + memberCredentials: + policy: allow-list + """ + " " + spellings[i][0] + ": " + spellings[i][1] + "\n"); + + FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials(); + assertEquals(spellings[i][2], credentials.sshAgentEnv(), + spellings[i][0] + ": " + spellings[i][1] + " must normalize correctly"); + assertEquals("inherit".equals(spellings[i][2]), credentials.sshAgentEnvInherited()); + } + } + + /** Protects the live {@code sshAuthSock: block} allow-list configuration during the rename. */ + @Test + void legacySshAuthSockBlockKeepsLiveAllowListConfigOmitted(@TempDir Path dir) throws Exception { + Path file = dir.resolve("live-member-credentials.yaml"); + Files.writeString(file, """ + bind: + port: 8080 + memberCredentials: + policy: allow-list + sshAuthSock: block + """); + + FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials(); + + assertEquals("omit", credentials.sshAgentEnv()); + assertFalse(credentials.sshAgentEnvInherited(), "the live config must omit SSH_AUTH_SOCK"); + } + + @Test + void sshAgentEnvWinsWhenBothCompatibleKeysArePresent(@TempDir Path dir) throws Exception { + Path file = dir.resolve("both-ssh-agent-keys.yaml"); + Files.writeString(file, """ + bind: + port: 8080 + memberCredentials: + policy: allow-list + sshAuthSock: allow + sshAgentEnv: omit + """); + + FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials(); + + assertEquals("omit", credentials.sshAgentEnv()); + assertFalse(credentials.sshAgentEnvInherited()); + } + + @Test + void sshAgentEnvDefaultsToOmitAndUnknownValuesFailClosed(@TempDir Path dir) throws Exception { + Path absent = dir.resolve("member-credentials-ssh-agent-absent.yaml"); + Files.writeString(absent, """ + bind: + port: 8080 + memberCredentials: + policy: allow-list + """); + assertEquals("omit", FleetConfig.load(absent).memberCredentials().sshAgentEnv()); + + Path unknown = dir.resolve("member-credentials-ssh-agent-unknown.yaml"); + Files.writeString(unknown, """ + bind: + port: 8080 + memberCredentials: + policy: allow-list + sshAgentEnv: inhert + """); + FleetConfig.MemberCredentials credentials = FleetConfig.load(unknown).memberCredentials(); + assertEquals("omit", credentials.sshAgentEnv()); + assertFalse(credentials.sshAgentEnvInherited(), "an unknown value must fail closed"); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java index 75f883c..e77cb41 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -170,10 +170,10 @@ class HerdrPeerLauncherAllowListWiringTest { /** * {@code SSH_AUTH_SOCK} is a live ssh-agent handle, not a value — it must stay blocked under * {@code allow-list} even when the operator lists it under {@code allow:}, because {@code - * sshAuthSock} defaults to blocked. Governed ONLY by {@code memberCredentials.sshAuthSock}. + * sshAgentEnv} defaults to omit. Governed ONLY by {@code memberCredentials.sshAgentEnv}. */ @Test - void sshAuthSockStaysBlockedEvenWhenListedInMemberCredentialsAllow() { + void sshAgentEnvStaysOmittedEvenWhenListedInMemberCredentialsAllow() { FakeHerdr herdr = new FakeHerdr(); WiringLauncher launcher = new WiringLauncher(herdr, allowListWithAllow(List.of("SSH_AUTH_SOCK"))); @@ -184,7 +184,7 @@ class HerdrPeerLauncherAllowListWiringTest { String scrub = readAll(dir.resolve(EnvAllowListScrub.SCRUB_FILE)); assertFalse(scrub.contains("'SSH_AUTH_SOCK'"), "SSH_AUTH_SOCK must not be on the derived allow-list just because the operator put " - + "it under allow: — sshAuthSock is unset here, so it defaults to block"); + + "it under allow: — sshAgentEnv is unset here, so it defaults to omit"); } @Test diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java index bc0f9a7..d579419 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/MemberEnvAllowListTest.java @@ -109,11 +109,11 @@ class MemberEnvAllowListTest { /** * {@code SSH_AUTH_SOCK} is a live handle to the operator's ssh-agent, never a value — so it must * stay excluded from the derived set even when the operator lists it under {@code allow:} for an - * unrelated reason. It is governed ONLY by {@code memberCredentials.sshAuthSock}, applied + * unrelated reason. It is governed ONLY by {@code memberCredentials.sshAgentEnv}, applied * separately by the caller ({@code HerdrPeerLauncher}). */ @Test - void sshAuthSockInMemberCredentialsAllowIsStillExcluded() { + void sshAgentEnvInMemberCredentialsAllowIsStillExcluded() { Set derived = MemberEnvAllowList.derive(List.of(), Set.of("SSH_AUTH_SOCK", "OTHER_NAME")); assertFalse(derived.contains("SSH_AUTH_SOCK"),