From 29d3f0b41fb7987b5f9feea7d4b690f8cd4351e9 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Wed, 9 Sep 2026 07:49:02 +0700 Subject: [PATCH] t377: report the git host value's SHAPE at startup, never its value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A member receives the git host as GITEA_HOST and often gets a full URL (scheme, trailing slash) where it expects a bare host, so it builds https://https://... and the request never leaves the machine. Logs set/unset, length, startsWithScheme and trailingSlash next to the existing secret report. The value itself is never logged, and the value is still passed to members unchanged — rewriting it here would change what works on one host and breaks on another. Written by the gx member on branch worker/t377-7f587b-7; it could not build, commit or push because the host command classifier refused every shell command (see #381). Build and verification are mine. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 75 ++++++ .../ltms/fleet/GitHostShapeReportTest.java | 221 ++++++++++++++++++ 2 files changed, 296 insertions(+) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/GitHostShapeReportTest.java diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 86370cf..a52227e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -123,6 +123,11 @@ public final class Fleetd { // else can fail on a silently-empty one. A daemon started without a login shell (launchd) // boots fine either way — this is the only thing that says so out loud. reportRequiredSecrets(cfg); + // fleetd #377: the git host value a member receives as GITEA_HOST is often a full URL + // (scheme and trailing slash), not a host name. A member that assumes a bare host then + // builds https://https://... and the request never leaves the machine. Report the shape + // next to the secret report — shape only, never the value. + reportGitHostShape(cfg); reportMemberTrustModel(cfg); // CB-596: an absent (or empty) memberCredentials: block blocks NOTHING — no credential // name is hardcoded any more to fall back on. Say so loudly, the same way a missing @@ -1195,6 +1200,76 @@ public final class Fleetd { }); } + /** + * fleetd #377: the env var names holding the git host value that members receive as + * {@code GITEA_HOST}. {@code HerdrPeerLauncher.applyGitToken} injects {@code GITEA_HOST} + * only for profiles that opted in via {@code gitTokenEnv} (CB-302), reading the value from + * that profile's {@code gitHostEnv} (default {@code GITEA_HOST}), so the shape only matters + * where a git token is opted in. A var used by more than one profile is one entry naming + * every profile that reads it, the same shape as {@link #requiredSecretEnvVars}. + * + *

Package-private and pure (no I/O, no logging) so the derivation is unit-testable + * without capturing log output; {@link #reportGitHostShape(FleetConfig)} is the logging caller. + */ + static Map> gitHostEnvVars(FleetConfig cfg) { + Map> hostsBy = new LinkedHashMap<>(); + cfg.profiles().forEach((name, profile) -> { + if (profile.hasGitToken()) { + hostsBy.computeIfAbsent(profile.gitHostEnv(), _ -> new ArrayList<>()) + .add("profile '" + name + "' gitHostEnv"); + } + }); + return hostsBy; + } + + /** + * True when the value already starts with a URI scheme ({@code https://...}, {@code + * http://...}). A bare host name and a host:port must both report {@code false} — the shape + * this ticket exists for is a value that looks like a host but is a full URL, and + * confusing those in the report would move the failure to the log instead of the network. + */ + static boolean startsWithScheme(String value) { + return value.matches("[A-Za-z][A-Za-z0-9+.-]*://.*"); + } + + /** + * fleetd #377: log, on the same startup path as {@link #reportRequiredSecrets}, the SHAPE of + * each git host value that members receive as {@code GITEA_HOST}: set or unset, its length, + * whether it starts with a scheme, whether it ends with a slash. Never the value itself — the + * same discipline as {@link #reportRequiredSecrets}, which logs by name only. Either shape is + * legitimate: the value is passed to members unchanged, and a line that quietly rewrites it + * would change what works on one host and breaks on another. The shape line only tells the + * operator which URL form to expect from a member that builds on {@code GITEA_HOST}. An + * unset variable is logged at INFO — useful information, not an error — and the daemon + * starts on either way. + * + *

The env read lives in the overload below so a test can drive the line with a known + * value and prove that value never reaches the log. + */ + static void reportGitHostShape(FleetConfig cfg) { + reportGitHostShape(cfg, System.getenv()); + } + + static void reportGitHostShape(FleetConfig cfg, Map env) { + Map> hostsBy = gitHostEnvVars(cfg); + if (hostsBy.isEmpty()) { + log.info("startup git host: no profile sets a gitTokenEnv — nothing to check"); + return; + } + hostsBy.forEach((varName, sources) -> { + String value = env.get(varName); + if (value == null || value.isBlank()) { + log.info("startup git host {}: unset ({}) — a member gets GITEA_TOKEN but no " + + "GITEA_HOST value", varName, String.join(", ", sources)); + } else { + log.info("startup git host {}: set ({}) — length={}, startsWithScheme={}, " + + "trailingSlash={}", + varName, String.join(", ", sources), + value.length(), startsWithScheme(value), value.endsWith("/")); + } + }); + } + /** * fleetd #184: state the member trust model at startup. Environment controls and worktrees do * not make a sandbox when fleetd and its members use the same OS user. A separate herdr may diff --git a/fleetd/src/test/java/dev/ltms/fleet/GitHostShapeReportTest.java b/fleetd/src/test/java/dev/ltms/fleet/GitHostShapeReportTest.java new file mode 100644 index 0000000..fdb6ac3 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/GitHostShapeReportTest.java @@ -0,0 +1,221 @@ +package dev.ltms.fleet; + +import ch.qos.logback.classic.Level; +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 org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.slf4j.LoggerFactory; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * fleetd #377: the startup git host line must report the shape of the value a + * member receives as GITEA_HOST — set or unset, length, scheme, trailing slash — and the + * value itself must never reach the log. + */ +class GitHostShapeReportTest { + + /** A profile that opts in to the git-forge token, so GITEA_HOST is the var that matters. */ + private static final String GIT_TOKEN_CONFIG = """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + gitTokenEnv: WORKER_GITEA_TOKEN + """; + + private static FleetConfig load(Path dir, String yaml) throws Exception { + Path f = dir.resolve("fleetd.yaml"); + Files.writeString(f, yaml); + return FleetConfig.load(f); + } + + private static ListAppender attach() { + Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + return appender; + } + + private static void detach(ListAppender appender) { + ((Logger) LoggerFactory.getLogger(Fleetd.class)).detachAppender(appender); + } + + private static List messages(ListAppender appender) { + return appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList(); + } + + @Test + void setLineReportsLengthSchemeAndTrailingSlash(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, GIT_TOKEN_CONFIG); + String value = "https://git.example.test/"; + + ListAppender appender = attach(); + try { + Fleetd.reportGitHostShape(cfg, Map.of("GITEA_HOST", value)); + } finally { + detach(appender); + } + + String expected = "startup git host GITEA_HOST: set (profile 'local' gitHostEnv) — " + + "length=" + value.length() + ", startsWithScheme=true, trailingSlash=true"; + assertTrue(messages(appender).contains(expected), + "a value with a scheme and a trailing slash must be reported by shape only: " + + "its length, startsWithScheme=true, trailingSlash=true"); + } + + @Test + void bareHostLineReportsNoSchemeNoTrailingSlash(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, GIT_TOKEN_CONFIG); + String value = "git.example.test"; + + ListAppender appender = attach(); + try { + Fleetd.reportGitHostShape(cfg, Map.of("GITEA_HOST", value)); + } finally { + detach(appender); + } + + String expected = "startup git host GITEA_HOST: set (profile 'local' gitHostEnv) — " + + "length=" + value.length() + ", startsWithScheme=false, trailingSlash=false"; + assertTrue(messages(appender).contains(expected), + "a bare host name must report startsWithScheme=false, trailingSlash=false"); + } + + @Test + void unsetVariableIsLoggedAtInfoAndDoesNotThrow(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, GIT_TOKEN_CONFIG); + + ListAppender appender = attach(); + try { + // GITEA_HOST absent from the map entirely — the unset case must not throw. + Fleetd.reportGitHostShape(cfg, Map.of()); + } finally { + detach(appender); + } + + assertTrue(appender.list.stream().anyMatch(e -> + e.getLevel() == Level.INFO + && e.getFormattedMessage() + .startsWith("startup git host GITEA_HOST: unset (profile 'local' gitHostEnv)")), + "an unset git host is useful information, not an error — say it at INFO level"); + } + + /** + * The important test: the value must NEVER appear in the log output. This test fails if the + * line is ever changed to include the value, because it drives the real logging path with a + * value that carries a marker no shape field could contain. + */ + @Test + void theValueNeverAppearsInLogOutput(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, GIT_TOKEN_CONFIG); + String marker = "never-logged-host-shape-377"; + String value = "https://" + marker + "/"; + + ListAppender appender = attach(); + try { + Fleetd.reportGitHostShape(cfg, Map.of("GITEA_HOST", value)); + } finally { + detach(appender); + } + + List msgs = messages(appender); + assertFalse(msgs.stream().anyMatch(m -> m.contains(value)), + "the full GITEA_HOST value must never reach the log"); + assertFalse(msgs.stream().anyMatch(m -> m.contains(marker)), + "no fragment of the value may reach the log — a line that embeds the value" + + " would leak at least this marker"); + assertTrue(msgs.stream().anyMatch(m -> m.contains("startsWithScheme=true") + && m.contains("trailingSlash=true")), + "the shape must still be reported although the value is not"); + } + + @Test + void unsetReportAlsoCarriesNoValue(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, GIT_TOKEN_CONFIG); + // A blank value is not injected either (putIfPresent skips it), so it must report unset + // without echoing anything of it. + ListAppender appender = attach(); + try { + Fleetd.reportGitHostShape(cfg, Map.of("GITEA_HOST", " ")); + } finally { + detach(appender); + } + + assertTrue(messages(appender).stream().anyMatch(m -> + m.startsWith("startup git host GITEA_HOST: unset")), + "a blank value is skipped by the launcher, so the line reports unset"); + } + + @Test + void defaultGitHostEnvIsGITEA_HOST(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, GIT_TOKEN_CONFIG); + + assertEquals(Map.of("GITEA_HOST", List.of("profile 'local' gitHostEnv")), + Fleetd.gitHostEnvVars(cfg)); + } + + @Test + void explicitGitHostEnvReportsUnderItsOwnName(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + gitTokenEnv: WORKER_GITEA_TOKEN + gitHostEnv: MY_FORGE_HOST + """); + String value = "https://forge.example.test/"; + + ListAppender appender = attach(); + try { + Fleetd.reportGitHostShape(cfg, Map.of("MY_FORGE_HOST", value)); + } finally { + detach(appender); + } + + assertTrue(messages(appender).contains( + "startup git host MY_FORGE_HOST: set (profile 'local' gitHostEnv) — " + + "length=" + value.length() + + ", startsWithScheme=true, trailingSlash=true"), + "an explicitly named gitHostEnv is reported under that name"); + } + + @Test + void noGitTokenMeansNoHostLineIsNeeded(@TempDir Path dir) throws Exception { + FleetConfig cfg = load(dir, """ + profiles: + local: + baseUrl: http://gx00.gw:8000 + """); + + ListAppender appender = attach(); + try { + Fleetd.reportGitHostShape(cfg, Map.of()); + } finally { + detach(appender); + } + + assertEquals(List.of("startup git host: no profile sets a gitTokenEnv — nothing to check"), + messages(appender)); + } + + @Test + void aHostWithAPortIsNotTreatedAsAScheme() { + assertFalse(Fleetd.startsWithScheme("git.example.test")); + assertFalse(Fleetd.startsWithScheme("git.example.test:3000")); + assertFalse(Fleetd.startsWithScheme("git.example.test:3000/")); + assertTrue(Fleetd.startsWithScheme("https://git.example.test")); + assertTrue(Fleetd.startsWithScheme("https://git.example.test:3000/")); + assertTrue(Fleetd.startsWithScheme("ssh://git.example.test")); + } +} \ No newline at end of file