Compare commits

...

1 Commits

Author SHA1 Message Date
Dai Ha 32408d1e64 fleetd #509: pin the pane-scan completeness fold, and stop legacyPrincipal handing out primary
CI / contract (pull_request) Successful in 57s
CI / build (pull_request) Successful in 1m40s
Unit 1 — PaneLocator.terminalForPid's completeness fold across herdr
clients (PaneLocator.java:117) had no test that varied the number of
clients, so a mutation that keeps only the last client's Lookup.complete()
instead of ANDing every client's outcome survived: 14 of 15 existing tests
agree with the mutant on a single client. Added a two-client test where
the lead client errors on the pane that would have owned the pid (an
incomplete, negative scan) and the member client cleanly finds no panes
(a complete, negative scan) — the real fold ANDs these to false, a
last-wins fold reads it as true. Proved against MUTANTC
(complete = outcome.complete();): the new test fails with
"expected: <false> but was: <true>", the file was restored byte-identical
(sha256 unchanged), and the control run is green.

Unit 2 — FleetMcp.legacyPrincipal's else-branch returned Principal.primary
for ANY caller the connection did not resolve to a worker pane, with none
of CallerResolver.java:254's isLoopback/scanComplete guards. Measured that
no production caller passes null callers (Fleetd.java:696 always
constructs a real CallerResolver) but FleetMcpAuthzTest.mcp(false)
legitimately does, for its "legacy constructor leaves the gate open" test
— so the null-callers path is not dead code to delete (option a), it is a
documented legacy mode (option b). Changed the else-branch to
Principal.anonymous() and widened legacyPrincipal to package-private (like
denyFor) so a new test pins the behavior directly, since it only ever ran
inside a contextExtractor closure no existing test triggers.
2026-09-12 10:58:09 +07:00
3 changed files with 65 additions and 4 deletions
@@ -527,14 +527,28 @@ public final class FleetMcp {
}
/**
* Pre-CB-501 identity: worker if the connection maps to a pane, otherwise the primary. Used
* only by the legacy constructor, where authorization is not enforced anyway.
* Pre-CB-501 identity: worker if the connection maps to a pane, otherwise anonymous. Used
* only by the legacy constructor ({@code callers == null}), where authorization is not
* enforced anyway — but the resolved {@link Principal} still reaches non-authz logic (e.g.
* {@code markSpawnedMemberPresent}, {@code recordPrimarySingleton}), so it must not be trusted
* with a role it did not earn.
*
* <p>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.
*
* <p>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. */
@@ -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();
@@ -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 ----------------------------------
/**