fleetd #509: this used to fall back to {@link Principal#primary}, unconditionally, for + * every caller the connection did not resolve to a worker pane — with none of + * {@code CallerResolver.java:254}'s two guards ({@code isLoopback}, {@code scanComplete}). + * That is the exact shape #317 and #505 each closed on the enforced path; this branch was the + * same trap, left open on the legacy one. It now returns {@link Principal#anonymous} instead, + * so an unresolved legacy caller earns no authority rather than the primary's. + * + *
Package-private (was {@code private}) so this is unit-testable directly, the same reason + * {@link #denyFor} was split out — it runs inside a contextExtractor closure that only fires on + * a real MCP request, so nothing else could pin this behaviour. */ - private static Principal legacyPrincipal(ConnectionIdentity identity, String addr, int port) { + static Principal legacyPrincipal(ConnectionIdentity identity, String addr, int port) { ConnectionIdentity.Caller c = identity.resolve(addr, port); return c.terminal() != null ? Principal.worker(c.terminal(), c.pid()) - : Principal.primary(c.pid()); + : Principal.anonymous(); } /** The caller reconstructed from the transport context. */ diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java index 9818fae..e3fce0c 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java @@ -180,6 +180,32 @@ class PaneLocatorTest { assertTrue(outcome.complete(), "a positive match elsewhere in the scan is definitive"); } + // --- fleetd #509: the completeness fold across clients must not collapse to "last wins" ---- + + @Test + void anEarlierClientsErrorSurvivesALaterClientsCleanNegative() { + // terminalForPid folds each client's Lookup.complete() with + // complete = complete && outcome.complete(); + // (PaneLocator.java:117). With a SINGLE client, a fold that keeps only the last outcome + // (dropping the "complete &&" prefix) agrees with the real fold — which is why 14 of the + // 15 pre-existing tests never catch that mutation: none of them vary the number of clients. + // Here the LEAD client errors on exactly the pane that would have owned the pid (so its + // scan is incomplete AND finds no match), and the MEMBER client cleanly reports no panes + // at all (a complete, negative scan). The real fold ANDs the two into false. A fold that + // just keeps the last client's outcome would read this as a clean true — the earlier + // error is erased, and CallerResolver.java:254 would read scanComplete() as true and + // promote an unverified caller to the primary. + HerdrClient lead = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient"); + HerdrClient member = new FakeHerdr().withNoPanes(); + PaneLocator two = new PaneLocator(lead, member); + + PaneLocator.Lookup outcome = two.terminalForPid(FakeHerdr.WORKER_PID); + + assertNull(outcome.terminal(), "the pane that could have owned the pid was never checked"); + assertFalse(outcome.complete(), + "an earlier client's error must survive a later client's clean negative"); + } + /** Minimal single-pane {@link HerdrClient} fake, purpose-built for the ancestry tests above. */ private static final class OnePaneHerdr implements HerdrClient { private final ObjectMapper mapper = new ObjectMapper(); diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java index 38a26ab..d540948 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java @@ -190,6 +190,27 @@ class FleetMcpAuthzTest { "no CallerResolver supplied ⇒ authorization not enforced (legacy behaviour)"); } + /** + * fleetd #509: {@code legacyPrincipal} (used only when {@code callers == null}, i.e. the + * legacy constructor above) used to fall back to {@link Principal#primary} for ANY caller the + * connection did not resolve to a worker pane — no {@code isLoopback} check, no + * {@code scanComplete} check, unlike the enforced path's {@code CallerResolver.java:254}. A + * non-loopback caller (an off-host client) is exactly the case that must never earn the + * primary's authority, and authorization being disabled in legacy mode does not make that + * safe: the resolved {@link Principal} still reaches non-authz logic such as + * {@code markSpawnedMemberPresent} and {@code recordPrimarySingleton}. + */ + @Test + void legacyPrincipalIsAnonymousNotPrimaryForAnUnresolvedCaller() { + ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999); + // A non-loopback address never even reaches the pane scan — resolve() short-circuits it + // to Caller(null, -1, true), the same "no terminal" shape a genuine primary's connection + // produces. legacyPrincipal must not conflate the two. + Principal p = FleetMcp.legacyPrincipal(identity, "8.8.8.8", 1234); + assertEquals(Principal.anonymous(), p, + "an unresolved legacy caller must earn no authority, not the primary's"); + } + // --- fleetd #439: who may see fleet_list's coordinator row ---------------------------------- /**