diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java index 3143175..f2d5144 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java @@ -427,7 +427,7 @@ public final class FleetMcp { leadSeats, callers == null ? Map.of() : callers.leads(), callerTerminal(exchange), new CoordinationSource(leadChannel, peers), - principal(exchange).isPrimary()); + coordinatorVisibleTo(principal(exchange))); }; BiFunction stopHandler = (exchange, req) -> { @@ -580,6 +580,20 @@ public final class FleetMcp { } } + /** + * fleetd #439: only the primary may read {@code fleet_list}'s {@code coordinator} row — + * lead-to-lead coordination state (coord-ids, mailbox facts, held-message previews), never the + * roster. Split out of the {@code fleet_list} handler, same reason as {@link #denyFor} and + * {@link #recordPrimarySingleton}: the decision must be unit-testable without fabricating an + * SDK {@code McpSyncServerExchange}, and the handler must call this named predicate rather than + * inlining the check, so a future edit cannot silently pass a literal instead of asking who + * called ({@code FleetMcpAuthzTest.theFleetListHandlerActuallyConsultsCoordinatorVisibleTo} + * reads the source and asserts the handler calls this method by name, not a literal). + */ + static boolean coordinatorVisibleTo(Principal caller) { + return caller.isPrimary(); + } + /** The worker identity resolved from this call's connection, or {@code null} if the primary. */ private static String callerTerminal(McpSyncServerExchange exchange) { Object v = exchange.transportContext().get(CALLER_TERMINAL); 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 016037c..50f1b4c 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/FleetMcpAuthzTest.java @@ -187,6 +187,71 @@ class FleetMcpAuthzTest { "no CallerResolver supplied ⇒ authorization not enforced (legacy behaviour)"); } + // --- fleetd #439: who may see fleet_list's coordinator row ---------------------------------- + + /** + * fleetd #439: {@link FleetMcp#coordinatorVisibleTo} is the whole policy decision for + * {@code fleet_list}'s {@code coordinator} row — lead-to-lead coordination state, not roster + * observation. Only the primary may see it; a worker, an architect, and (the case the previous + * pass of this ticket did not cover) an anonymous caller must all be refused. + */ + @Test + void onlyThePrimaryMaySeeTheCoordinatorRow() { + assertTrue(FleetMcp.coordinatorVisibleTo(PRIMARY), "the primary must see its own coordination state"); + assertFalse(FleetMcp.coordinatorVisibleTo(WORKER_A), "a worker must not see lead-to-lead coordination state"); + assertFalse(FleetMcp.coordinatorVisibleTo(ARCH_DESIGN), + "an architect holds READ today, but that must not extend to coordinator"); + assertFalse(FleetMcp.coordinatorVisibleTo(ANON), "authenticated as nothing must not see it either"); + } + + /** + * fleetd #439 / PR #462 review finding M2: the predicate above can be perfectly correct while + * the one production call site (the {@code fleet_list} MCP handler) never actually asks it — + * a literal {@code true} compiles, and the whole suite stayed green under that mutation because + * every existing test drives {@link FleetMcp#listFleet} directly and supplies the boolean + * itself. This test reads {@code FleetMcp.java}'s own source (same idiom as {@link + * #toolsTheServerRegisters()} / {@link #everyRegisteredToolHasItsHandlerActionPinned()}) and + * asserts the handler's call passes {@code coordinatorVisibleTo(principal(exchange))} — not a + * literal {@code true} or {@code false} — as {@code listFleet}'s trailing argument. + * + *

Anchored on argument position, not a bare substring search: {@code true} appears many + * times elsewhere in this file for unrelated reasons, so a plain {@code contains("true")} + * check would prove nothing. The pattern requires the literal text immediately before the + * closing {@code );} of the {@code listFleet(} call inside the handler block to be exactly + * {@code coordinatorVisibleTo(principal(exchange))}. + */ + @Test + void theFleetListHandlerActuallyConsultsCoordinatorVisibleTo() throws Exception { + String source = Files.readString(MCP_SOURCE); + + // Isolate the fleet_list handler block: from its declaration up to the next handler's + // declaration. A change to variable naming would break this scrape loudly (see the control + // assertion just below), rather than silently reporting "no violation found". + int start = source.indexOf("listHandler ="); + assertTrue(start >= 0, "could not find the fleet_list handler (listHandler) in " + MCP_SOURCE + + " -- the scrape has stopped matching, fix the anchor before trusting this test"); + int end = source.indexOf("stopHandler =", start); + assertTrue(end > start, "could not find the handler declared after listHandler to bound the scrape"); + String handlerBlock = source.substring(start, end); + + // CONTROL: the block we scraped really does contain a call to listFleet(...) -- if this + // fails, the anchors above moved and the assertion below would otherwise pass on nothing. + assertTrue(handlerBlock.contains("listFleet("), + "control failed: the scraped listHandler block contains no listFleet( call at all -- " + + "the anchors have drifted, this test is not testing what it claims to"); + + Pattern trailingArg = Pattern.compile( + "listFleet\\([^;]*?,\\s*(coordinatorVisibleTo\\(principal\\(exchange\\)\\)|true|false)\\s*\\)\\s*;", + Pattern.DOTALL); + Matcher m = trailingArg.matcher(handlerBlock); + assertTrue(m.find(), "could not locate listFleet(...)'s trailing boolean argument in the " + + "listHandler block -- the call shape changed, update this test's anchor: " + handlerBlock); + String trailing = m.group(1); + assertEquals("coordinatorVisibleTo(principal(exchange))", trailing, + "the fleet_list handler must ask coordinatorVisibleTo(principal(exchange)) who is " + + "calling, not pass a literal boolean -- found: " + trailing); + } + // --- which action each tool hands the gate (fleetd #272) ------------------------------------ /**