Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 656588f597 | |||
| e33377b2ca | |||
| f288cee2bb |
@@ -2688,9 +2688,10 @@ public record FleetConfig(
|
||||
* tab labels — every member gets one rendered into its tab. Choose a lead {@code tabPrefix} that
|
||||
* a member template matches and the daemon starts labelling its own members as leads, promoting
|
||||
* the entire fleet to {@link dev.ltms.fleet.auth.Role#PRIMARY} with no message and no diff.
|
||||
* The member-space exclusion in {@link dev.ltms.fleet.herdr.LeadTabScanner} already blocks the
|
||||
* realistic path, but defence that depends on one workspace label holding is not defence enough
|
||||
* for a privilege boundary.
|
||||
* {@link #validatePanePlacementAgainstLeadTabs()} is the check that stops a pane-placed member
|
||||
* from landing inside a lead's tab in the first place; this check is a second, independent
|
||||
* guard that catches the hazard even when every profile places members correctly, by refusing
|
||||
* a label that a scan would still misread as a lead.
|
||||
*
|
||||
* <p>CB-557 shrank this check rather than removing it. The default template is
|
||||
* {@code "{role}: {profile} #{n}"} and {@code {role}} comes from a closed enum, so a
|
||||
@@ -2737,6 +2738,45 @@ public record FleetConfig(
|
||||
+ "lead tabs cannot be confused.");
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject a profile that places its members by {@code "pane"} while any {@code fleet.leaders}
|
||||
* entry names a {@code tab}. A pane-placed member lands inside the focused tab rather than its
|
||||
* own, so it can land inside a lead's own labelled tab. {@link
|
||||
* dev.ltms.fleet.herdr.LeadTabScanner} identifies a lead purely by that tab's label — it does
|
||||
* not exclude the member space — so a member that ends up there would be read back as the lead
|
||||
* and granted spawn/stop/send on the whole fleet.
|
||||
*
|
||||
* <p>Only a leader with a non-blank {@code tab} is in scope: one with no {@code tab} feeds
|
||||
* nothing into {@link dev.ltms.fleet.herdr.LeadTabScanner}, so it creates no hazard here.
|
||||
*
|
||||
* @throws IllegalStateException when any {@code profiles:} entry is pane-placed while any
|
||||
* {@code fleet.leaders} entry names a non-blank {@code tab}
|
||||
*/
|
||||
public void validatePanePlacementAgainstLeadTabs() {
|
||||
if (fleet == null || fleet.leaders().isEmpty()) {
|
||||
return;
|
||||
}
|
||||
boolean anyLeaderHasTab = fleet.leaders().values().stream()
|
||||
.anyMatch(leader -> leader != null && leader.tab() != null && !leader.tab().isBlank());
|
||||
if (!anyLeaderHasTab) {
|
||||
return;
|
||||
}
|
||||
List<String> bad = new ArrayList<>();
|
||||
profiles().entrySet().stream()
|
||||
.filter(e -> !e.getValue().tabPlacement())
|
||||
.map(Map.Entry::getKey)
|
||||
.sorted()
|
||||
.forEach(bad::add);
|
||||
if (bad.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
throw new IllegalStateException("refusing to start: profile(s) " + bad
|
||||
+ " use placement: pane while fleet.leaders names a tab. A pane-placed member can "
|
||||
+ "land inside a lead's labelled tab and be read back as the lead, granted "
|
||||
+ "spawn/stop/send on the whole fleet. Set placement: tab for each named profile, "
|
||||
+ "or remove the tab from every fleet.leaders entry.");
|
||||
}
|
||||
|
||||
/**
|
||||
* Reject a present {@code leadRollover:} block with no (or a blank) {@code handoverPath}
|
||||
* (fleetd #480). There is no sane non-null default for an operator-specific file path, unlike
|
||||
|
||||
@@ -37,8 +37,11 @@ import java.util.function.Supplier;
|
||||
* <p><strong>Direction of trust.</strong> The label names the lead; it never <em>grants</em>
|
||||
* anything a pane could take for itself. Three properties keep that honest:
|
||||
* <ol>
|
||||
* <li>Worker spaces are excluded wholesale ({@code excludedWorkspaceLabels}), so a worker cannot
|
||||
* become a lead by being placed — as a split, say — inside a matching tab.</li>
|
||||
* <li>{@code excludedWorkspaceLabels} can filter a workspace out of the scan, but this class does
|
||||
* not by itself stop a worker from landing inside a matching tab — a caller may pass an empty
|
||||
* set, and the daemon does. The guard against that is {@code
|
||||
* FleetConfig.validatePanePlacementAgainstLeadTabs}: it refuses, at startup, any profile that
|
||||
* places members by pane while a lead names a tab.</li>
|
||||
* <li>A worker cannot rename a tab: {@code tab.rename} is reachable only through
|
||||
* {@link WorkspaceControl}, which no {@code fleet_*} tool exposes. The label is writable by
|
||||
* the human at the terminal and by nobody the bridge is defending against.</li>
|
||||
|
||||
@@ -201,8 +201,8 @@ public final class LeadLauncher {
|
||||
/**
|
||||
* How many live leads exist per configured name, and which of that name's labelled tabs are
|
||||
* <em>not</em> live: a running agent in a tab labelled with that lead's exact {@code tab}
|
||||
* (CB-579). Member workspaces are excluded, exactly as the scanner excludes them: a member must
|
||||
* not be counted as a lead because it happens to sit in a matching tab.
|
||||
* (CB-579). A member sitting in the same shared workspace is not counted as a lead because its
|
||||
* tab carries a different label, not because any workspace is excluded from this count.
|
||||
*
|
||||
* <p>There used to be a second path here — a running agent on the terminal a
|
||||
* {@code fleet.leaders.<name>.terminal} pin named, for a lead opened and pinned by hand. That
|
||||
|
||||
@@ -765,6 +765,92 @@ class FleetConfigTest {
|
||||
"a label that collides with a convention nobody reads is not a problem");
|
||||
}
|
||||
|
||||
// ── validatePanePlacementAgainstLeadTabs ────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* The hazard this guard closes: a pane-placed member lands inside the focused tab rather than
|
||||
* its own, so it can land inside a lead's labelled tab and be read back as that lead.
|
||||
*/
|
||||
@Test
|
||||
void aPanePlacedProfileWithALeadTabRefusesToStart(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("pane-hazard.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
placement: pane
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead: opus"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class,
|
||||
cfg::validatePanePlacementAgainstLeadTabs);
|
||||
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
|
||||
}
|
||||
|
||||
@Test
|
||||
void aPanePlacedProfileWithNoLeadTabIsAllowed(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("pane-no-tab.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
placement: pane
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
profile: gx10
|
||||
""");
|
||||
|
||||
assertDoesNotThrow(() -> FleetConfig.load(f).validatePanePlacementAgainstLeadTabs(),
|
||||
"a leader with no tab feeds nothing into the scanner, so pane placement is safe");
|
||||
}
|
||||
|
||||
@Test
|
||||
void aTabPlacedProfileWithALeadTabIsAllowed(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("tab-safe.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
placement: tab
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead: opus"
|
||||
""");
|
||||
|
||||
assertDoesNotThrow(() -> FleetConfig.load(f).validatePanePlacementAgainstLeadTabs(),
|
||||
"a member in its own tab cannot land inside a lead's tab");
|
||||
}
|
||||
|
||||
/** Proves the reflective sweep behind {@code validateAll} really reaches this validator. */
|
||||
@Test
|
||||
void validateAllAlsoRefusesPanePlacementAgainstALeadTab(@TempDir Path dir) throws Exception {
|
||||
Path f = dir.resolve("pane-hazard-sweep.yaml");
|
||||
Files.writeString(f, """
|
||||
bind:
|
||||
port: 8080
|
||||
profiles:
|
||||
gx10:
|
||||
placement: pane
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead: opus"
|
||||
""");
|
||||
FleetConfig cfg = FleetConfig.load(f);
|
||||
|
||||
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateAll);
|
||||
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
|
||||
}
|
||||
|
||||
// ── CB-530/CB-579: the leaders registry ─────────────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
|
||||
@@ -42,12 +42,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
* has right now. This is the proof that a future, real seventh validator on {@link
|
||||
* FleetConfig} would be swept automatically, without needing to add a real (unwanted)
|
||||
* seventh validator just to exercise the claim.</li>
|
||||
* <li>{@link #validateAllReachesEveryOneOfTodaysSixValidators()} proves {@link
|
||||
* <li>{@link #validateAllReachesEveryOneOfTodaysRealValidators()} proves {@link
|
||||
* FleetConfig#validateAll()} itself is wired to that same generic mechanism and genuinely
|
||||
* reaches each of today's six real validators — reusing the exact minimal failing
|
||||
* reaches seven of today's eight real validators — reusing the exact minimal failing
|
||||
* configurations {@code FleetConfigTest} already established for each one directly, so a
|
||||
* single call to {@code validateAll()} is shown to reproduce every one of those six
|
||||
* failures.</li>
|
||||
* single call to {@code validateAll()} is shown to reproduce every one of those seven
|
||||
* failures. The eighth, {@link FleetConfig#validateLeadRollover()}, has no case here yet —
|
||||
* a pre-existing gap tracked as fleetd #668.</li>
|
||||
* </ol>
|
||||
*
|
||||
* <p>Together with the direct-{@code Fleetd.main}-invocation tests in {@code
|
||||
@@ -62,8 +63,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
* the generic sweep — claim 1 proves {@link FleetConfig#invokeAllValidators} is generic, and claim
|
||||
* 2 proves {@code validateAll()} reaches today's six, and a hardcoded list satisfies both. So the
|
||||
* reflective sweep is a convenience, not the guarantee. The guarantee is {@link
|
||||
* #fleetConfigDeclaresExactlyTheseSixValidatorsToday()}: it fails the moment a seventh validator
|
||||
* is declared, which forces whoever adds it to look at this file.
|
||||
* #fleetConfigDeclaresExactlyTheseValidatorsToday()}: it fails the moment any validator is added
|
||||
* or removed, which forces whoever changes the set to look at this file.
|
||||
*/
|
||||
class FleetConfigValidateAllTest {
|
||||
|
||||
@@ -209,18 +210,19 @@ class FleetConfigValidateAllTest {
|
||||
+ "name) must all be skipped");
|
||||
}
|
||||
|
||||
// ── Claim 2: FleetConfig.validateAll() is wired to that mechanism and reaches all six today ──
|
||||
// ── Claim 2: FleetConfig.validateAll() is wired to that mechanism and reaches seven of eight today ──
|
||||
|
||||
/**
|
||||
* Reflectively enumerates {@link FleetConfig}'s own public, no-arg, void {@code validateXxx()}
|
||||
* methods (excluding {@code validateAll} itself) — the exact same filter {@link
|
||||
* FleetConfig#invokeAllValidators} applies. This is not the mechanism proof (that is claim 1,
|
||||
* above, on an unrelated class) — it is a visible denominator: today there are six, named
|
||||
* here, so a reader adding a seventh sees this assertion name the new count rather than a
|
||||
* silent pass at the old one.
|
||||
* above, on an unrelated class) — it is a visible denominator: today there are eight, named in
|
||||
* the {@code Set.of} below, so a reader adding or removing one sees this assertion name the new
|
||||
* count rather than a silent pass at the old one. The count lives only in that set, not in this
|
||||
* method's name, so the two cannot drift apart.
|
||||
*/
|
||||
@Test
|
||||
void fleetConfigDeclaresExactlyTheseSixValidatorsToday() {
|
||||
void fleetConfigDeclaresExactlyTheseValidatorsToday() {
|
||||
Set<String> names = new TreeSet<>();
|
||||
for (Method m : FleetConfig.class.getMethods()) {
|
||||
if (java.lang.reflect.Modifier.isPublic(m.getModifiers())
|
||||
@@ -233,7 +235,8 @@ class FleetConfigValidateAllTest {
|
||||
}
|
||||
assertEquals(new TreeSet<>(Set.of("validateAuthExposure", "validateLeadTabPrefixes",
|
||||
"validateSubscriptionProfiles", "validateCharters", "validateMembers",
|
||||
"validateModels", "validateLeadRollover")), names,
|
||||
"validateModels", "validateLeadRollover", "validatePanePlacementAgainstLeadTabs")),
|
||||
names,
|
||||
"FleetConfig's public validate*() methods changed. Do TWO things, in this "
|
||||
+ "order. First confirm validateAll() still delegates to "
|
||||
+ "invokeAllValidators(this) — a hardcoded list there passes every other "
|
||||
@@ -262,14 +265,17 @@ class FleetConfigValidateAllTest {
|
||||
}
|
||||
|
||||
/**
|
||||
* The heart of claim 2: for each of today's six real validators, a minimal file that fails
|
||||
* The heart of claim 2: for seven of today's eight real validators, a minimal file that fails
|
||||
* ONLY that one — the exact fixtures {@code FleetConfigTest} uses to test each validator
|
||||
* directly — must also fail through {@link FleetConfig#validateAll()}. If a future edit to
|
||||
* {@code validateAll()} silently dropped one validator from the sweep (e.g. a typo'd name
|
||||
* filter), exactly one of these six would start passing when it must not.
|
||||
* {@code validateAll()} silently dropped one of these seven from the sweep (e.g. a typo'd name
|
||||
* filter), exactly one of them would start passing when it must not.
|
||||
*
|
||||
* <p>The eighth, {@link FleetConfig#validateLeadRollover()}, has no case here — a pre-existing
|
||||
* gap tracked as fleetd #668, not fixed by this change.
|
||||
*/
|
||||
@Test
|
||||
void validateAllReachesEveryOneOfTodaysSixValidators(@TempDir Path dir) throws Exception {
|
||||
void validateAllReachesEveryOneOfTodaysRealValidators(@TempDir Path dir) throws Exception {
|
||||
// validateAuthExposure: a non-loopback bind without token mode.
|
||||
assertValidateAllRefuses(dir, "auth-exposure.yaml", """
|
||||
bind:
|
||||
@@ -342,6 +348,20 @@ class FleetConfigValidateAllTest {
|
||||
allow:
|
||||
- model: claude-sonnet-5
|
||||
""", "rogue");
|
||||
|
||||
// validatePanePlacementAgainstLeadTabs: a pane-placed profile while a lead names a tab.
|
||||
assertValidateAllRefuses(dir, "pane-placement.yaml", """
|
||||
bind:
|
||||
host: 127.0.0.1
|
||||
port: 8765
|
||||
profiles:
|
||||
gx10:
|
||||
placement: pane
|
||||
fleet:
|
||||
leaders:
|
||||
opus:
|
||||
tab: "lead: opus"
|
||||
""", "gx10");
|
||||
}
|
||||
|
||||
private static void assertValidateAllRefuses(Path dir, String fileName, String yaml,
|
||||
|
||||
@@ -232,8 +232,9 @@ class LeadTabScannerTest {
|
||||
|
||||
@Test
|
||||
void everyPaneInALeadTabResolvesAsThatLead() {
|
||||
// A human may split their own lead tab. Both panes are theirs, so both are that lead —
|
||||
// nothing fleetd placed can land here (see the worker-space test above).
|
||||
// A human may split their own lead tab. Both panes are theirs, so both are that lead.
|
||||
// A pane-placed member landing here instead is refused at startup by
|
||||
// FleetConfig.validatePanePlacementAgainstLeadTabs, not by this scanner.
|
||||
TopologyHerdr herdr = twoLeads().pane("w1:p1b", "w1:t1", "term_opus_split");
|
||||
|
||||
assertEquals("opus-5.0",
|
||||
|
||||
Reference in New Issue
Block a user