CB-606: refuse an unrecognized auth.mode, per-profile placement, or top-level placement policy at config load
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.
This commit is contained in:
@@ -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<String> 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.
|
||||
*
|
||||
* <p>{@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<String> 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.
|
||||
*
|
||||
* <p>{@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<String> 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.
|
||||
*
|
||||
* <p>{@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<String> unknownTopLevelKeys(String yaml) {
|
||||
Map<?, ?> raw;
|
||||
try {
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user