From bb750cdba3b61da82544261cec6a25c9c66a318f Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sun, 16 Aug 2026 18:54:35 +0200 Subject: [PATCH] CB-606: refuse an unrecognized auth.mode, per-profile placement, or top-level placement policy at config load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An auth.mode typo (e.g. "toekn") used to silently fall back to loopback-trust with no signal anywhere — validateAuthExposure() only checks the pairing on a non-loopback bind, so on the common loopback bind the daemon started cleanly and authenticated nobody. Per-profile placement had the same shape, falling back to legacy pane placement. The top-level placement policy name was already validated by PlacementPolicies.fromName, but only lazily at first spawn through CompositePeerLauncher's Supplier; it is now checked eagerly at load, calling fromName itself as the single source of truth. --- .../ltms/bridged/config/BridgedConfig.java | 116 ++++++++++++++++++ .../bridged/config/BridgedConfigTest.java | 83 +++++++++++++ 2 files changed, 199 insertions(+) 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 c41d6f0..a6cac35 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import dev.ltms.bridged.msg.AmqpReplyInbox; import dev.ltms.bridged.peer.MemberRole; +import dev.ltms.bridged.placement.PlacementPolicies; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -987,7 +988,13 @@ public record BridgedConfig( rejectDuplicateMemberSlots(yaml); rejectNegativeMaxLoad(yaml); rejectUnknownKind(yaml); + rejectUnknownAuthMode(yaml); + rejectUnknownPlacement(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 + // Supplier), where a bad name would still start a daemon that looks healthy. + rejectUnknownPlacementPolicy(cfg.placement()); return cfg.withDefaults(); } catch (IOException e) { throw new UncheckedIOException("cannot read bridged config at " + path, e); @@ -1328,6 +1335,115 @@ public record BridgedConfig( } } + /** The auth modes this build understands — {@link Auth#mode()}'s only valid values. */ + private static final Set KNOWN_AUTH_MODES = Set.of(Auth.MODE_LOOPBACK_TRUST, Auth.MODE_TOKEN); + + /** + * Reject an {@code auth.mode} that is not one of {@link #KNOWN_AUTH_MODES} (CB-606), naming the + * value and the accepted set. + * + *

{@link Auth}'s compact constructor only lower-cases {@code mode}, and + * {@link Auth#tokenMode()} only compares the result against {@code MODE_TOKEN} — anything else, + * including a typo like {@code toekn}, silently behaves as {@code loopback-trust}. That fallback + * is otherwise checked only by {@link #validateAuthExposure()}, and only when the bind is + * non-loopback: on a loopback bind (the common case) the typo is invisible end to end — the + * daemon starts cleanly and authenticates nobody while the operator believes {@code token} mode + * is active. Refuse it here, unconditionally, at config load, rather than let it hide behind the + * bind check. + * + * @param yaml the raw config text + * @throws IllegalStateException when {@code auth.mode} is a non-blank value not in + * {@link #KNOWN_AUTH_MODES} (case-insensitive) + */ + static void rejectUnknownAuthMode(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("auth") instanceof Map auth)) { + return; + } + if (!(auth.get("mode") instanceof String mode) || mode.isBlank() + || KNOWN_AUTH_MODES.contains(mode.toLowerCase())) { + return; + } + throw new IllegalStateException("refusing to start: auth.mode=" + mode + + " is not recognized — accepted values are " + + String.join(", ", KNOWN_AUTH_MODES.stream().sorted().toList()) + + " (case-insensitive); an unrecognized mode would otherwise silently fall back to" + + " loopback-trust, which authenticates nobody."); + } + + /** The per-profile placements this build understands — {@link Profile#placement()}'s only valid values. */ + private static final Set KNOWN_PLACEMENTS = Set.of("tab", "pane"); + + /** + * Reject a profile whose {@code placement:} is not one of {@link #KNOWN_PLACEMENTS} (CB-606), + * naming the profile, the value it set, and the accepted set. + * + *

{@link Profile}'s compact constructor only lower-cases {@code placement}, and + * {@link Profile#tabPlacement()} only compares the result against {@code "tab"} — anything else, + * including a typo like {@code tabb}, silently falls back to the legacy pane placement with no + * signal anywhere. + * + * @param yaml the raw config text + * @throws IllegalStateException when any profile's {@code placement} is a non-blank value not in + * {@link #KNOWN_PLACEMENTS} (case-insensitive) + */ + static void rejectUnknownPlacement(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("profiles") instanceof Map profiles)) { + return; + } + List bad = profiles.entrySet().stream() + .filter(e -> e.getValue() instanceof Map p + && p.get("placement") instanceof String pl && !pl.isBlank() + && !KNOWN_PLACEMENTS.contains(pl.toLowerCase())) + .map(e -> String.valueOf(e.getKey()) + "=" + ((Map) e.getValue()).get("placement")) + .sorted() + .toList(); + if (!bad.isEmpty()) { + throw new IllegalStateException("refusing to start: profile(s) [" + String.join(", ", bad) + + "] set an unrecognized placement — accepted values are " + + String.join(", ", KNOWN_PLACEMENTS.stream().sorted().toList()) + + " (case-insensitive); an unrecognized placement would otherwise fall back to" + + " legacy pane placement with no signal anywhere."); + } + } + + /** + * 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. + * + *

{@code CompositePeerLauncher} only calls {@link PlacementPolicies#fromName} per spawn, + * through a {@code Supplier} that re-reads live config (CB-559, so a hot-reloaded placement + * policy takes effect without a restart) — so a bad name still starts a daemon that looks + * healthy and fails only the first time something spawns without naming a profile. Every other + * field this class validates fails here, at load; this one gets the same treatment, calling + * {@link PlacementPolicies#fromName} itself as the single source of truth for what is valid + * rather than duplicating its accepted set. + * + * @param placement the raw, possibly null/blank {@code placement} value as parsed (before + * {@link #withDefaults()} runs); {@code fromName} itself treats null/blank as + * {@code fixed}, so this call changes no default + * @throws IllegalStateException when {@code placement} is a name {@link PlacementPolicies} does + * not recognize + */ + private static void rejectUnknownPlacementPolicy(String placement) { + try { + PlacementPolicies.fromName(placement); + } catch (IllegalArgumentException e) { + throw new IllegalStateException("refusing to start: " + e.getMessage(), e); + } + } + static List unknownTopLevelKeys(String yaml) { Map raw; try { 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 8e2efa6..5ccb2d9 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -1407,6 +1407,89 @@ class BridgedConfigTest { assertTrue(e.getMessage().contains("maxLoad"), "error names the key: " + e.getMessage()); } + /** + * CB-606: an unrecognized {@code auth.mode} used to silently fall back to + * {@code loopback-trust} — {@link BridgedConfig.Auth#tokenMode()} only checked equality + * against {@code "token"}. On a loopback bind {@link BridgedConfig#validateAuthExposure()} + * never runs (it only fires for a non-loopback bind), so the typo was completely invisible: + * the daemon started cleanly and authenticated nobody while the operator believed token mode + * was active. + */ + @Test + void unknownAuthModeIsRefusedAtLoadNamingTheValueAndTheAcceptedSet(@TempDir Path dir) throws Exception { + Path f = dir.resolve("auth-mode-typo.yaml"); + Files.writeString(f, """ + bind: + host: 127.0.0.1 + port: 8765 + auth: + mode: toekn + """); + + IllegalStateException e = assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("toekn"), "error names the bad value: " + e.getMessage()); + assertTrue(e.getMessage().contains("loopback-trust") && e.getMessage().contains("token"), + "error names the accepted set: " + e.getMessage()); + } + + /** + * CB-606: an unrecognized per-profile {@code placement:} used to silently fall back to legacy + * pane placement — {@link BridgedConfig.Profile#tabPlacement()} only checked equality against + * {@code "tab"}. + */ + @Test + void unknownProfilePlacementIsRefusedAtLoadNamingTheProfileAndTheAcceptedSet(@TempDir Path dir) throws Exception { + Path f = dir.resolve("placement-typo.yaml"); + Files.writeString(f, """ + profiles: + gx10: + baseUrl: http://gx10.gw:8000 + placement: tabb + """); + + IllegalStateException e = assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("gx10"), "error names the profile: " + e.getMessage()); + assertTrue(e.getMessage().contains("tabb"), "error names the bad value: " + e.getMessage()); + assertTrue(e.getMessage().contains("tab") && e.getMessage().contains("pane"), + "error names the accepted set: " + e.getMessage()); + } + + @Test + void absentProfilePlacementDefaultsToTab(@TempDir Path dir) throws Exception { + Path f = dir.resolve("placement-absent.yaml"); + Files.writeString(f, """ + profiles: + gx10: + baseUrl: http://gx10.gw:8000 + """); + + BridgedConfig cfg = BridgedConfig.load(f); + BridgedConfig.Profile w = cfg.profiles().get("gx10"); + assertTrue(w.tabPlacement(), "an absent placement must keep defaulting to tab"); + } + + /** + * CB-606: the top-level {@code placement:} policy name WAS validated, but only lazily, by + * {@code PlacementPolicies.fromName} through {@code CompositePeerLauncher}'s per-spawn + * {@code Supplier} — so a bad name still started a daemon that looked healthy and failed only + * the first time something spawned without naming a profile. This must now fail at load. + */ + @Test + void unknownTopLevelPlacementPolicyIsRefusedAtLoadNotLazilyAtFirstSpawn(@TempDir Path dir) throws Exception { + Path f = dir.resolve("placement-policy-typo.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + placement: weightd + """); + + IllegalStateException e = assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("weightd"), "error names the bad value: " + e.getMessage()); + assertTrue(e.getMessage().contains("fixed") && e.getMessage().contains("round-robin") + && e.getMessage().contains("weighted"), + "error names the accepted set: " + e.getMessage()); + } + @Test void subscriptionFlagBindsAndDefaultsFalse(@TempDir Path dir) throws Exception { Path f = dir.resolve("subscription.yaml");